From: Thomas Gleixner <tglx@linutronix.de>
To: "Sergio González Collado" <sergio.collado@gmail.com>,
"Ingo Molnar" <mingo@redhat.com>,
"Borislav Petkov" <bp@alien8.de>,
"Dave Hansen" <dave.hansen@linux.intel.com>,
"H . Peter Anvin" <hpa@zytor.com>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Alex Gaynor" <alex.gaynor@gmail.com>,
"Wedson Almeida Filho" <wedsonaf@gmail.com>
Cc: "David Rheinsberg" <david@readahead.eu>,
"John Baublitz" <john.m.baublitz@gmail.com>,
"Danilo Krummrich" <danilokrummrich@yahoo.de>,
x86@kernel.org, "Boqun Feng" <boqun.feng@gmail.com>,
"Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <benno.lossin@proton.me>,
"Andreas Hindborg" <a.hindborg@samsung.com>,
"Alice Ryhl" <aliceryhl@google.com>,
rust-for-linux@vger.kernel.org,
"Sergio González Collado" <sergio.collado@gmail.com>
Subject: Re: [PATCH] Add room for insn-enconding and symbol name
Date: Tue, 14 May 2024 18:31:13 +0200 [thread overview]
Message-ID: <87seykl5im.ffs@tglx> (raw)
In-Reply-To: <20240414180929.79033-1-sergio.collado@gmail.com>
Sergio!
On Sun, Apr 14 2024 at 20:09, Sergio González Collado wrote:
Please read through Documentation/process/maintainers-tip.rst. It
explains details about how change logs and patch subjects should be
written.
> The longest length of a symbol (KSYM_NAME_LEN) was increased to 512
> in the reference [1]. This patch adds room for insn-encoding and a
> symbol name, as proposed in [2]
This is really not a good explanation. The changelog wants to be self
contained and explain the problem to be solved. Having to follow random
archive links to get an explanation is just painful. The links can only
serve as additional material.
> [1] https://lore.kernel.org/lkml/20220802015052.10452-6-ojeda@kernel.org/
> [2] https://lore.kernel.org/lkml/16641a56-9139-4396-a5a8-89606bedd1f1@app.fastmail.com/
>
> Signed-off-by: Sergio González Collado <sergio.collado@gmail.com>
> ---
> arch/x86/tools/insn_decoder_test.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/tools/insn_decoder_test.c b/arch/x86/tools/insn_decoder_test.c
> index 472540aeabc2..add51bfc2260 100644
> --- a/arch/x86/tools/insn_decoder_test.c
> +++ b/arch/x86/tools/insn_decoder_test.c
> @@ -10,6 +10,7 @@
> #include <assert.h>
> #include <unistd.h>
> #include <stdarg.h>
> +#include <linux/kallsym.h>
Please put a newline between the existing includes and the linux
prefixed one.
> #define unlikely(cond) (cond)
>
> @@ -106,7 +107,7 @@ static void parse_args(int argc, char **argv)
> }
> }
>
> -#define BUFSIZE 256
> +#define BUFSIZE (256 + KSYM_NAME_LEN)
What's the rationale here?
256 + KSYM_NAME_LEN is as arbitrary as defining the buffer size to
4096, i.e. it's a number pulled out of thin air.
If you really want to make this code resilent against future line length
changes then the obvious thing to do is:
static char *update_buf(char *buf, ssize_t size)
{
buf = realloc(buf, size);
if (!buf) {
complain();
exit(9);
}
}
main()
{
char *buffer = NULL, *sym = NULL, *copy = NULL;
ssize_t size = 256, oldsize = 0, len;
for (len = getline(&buffer, &size, stdin); len > 0;
len = getline(&buffer, &size, stdin) {
if (size != oldsize) {
update_buf(sym, size);
update_buf(copy, size);
}
buffer[len - 1] = '\0';
...
}
if (errno)
complain(errno);
free(buffer);
free(sym);
free(copy);
...
No?
Thanks,
tglx
prev parent reply other threads:[~2024-05-14 16:31 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-14 18:09 [PATCH] Add room for insn-enconding and symbol name Sergio González Collado
2024-05-08 11:58 ` Danilo Krummrich
2024-05-14 12:56 ` Danilo Krummrich
2024-05-14 13:01 ` Sergio González Collado
2024-05-14 16:31 ` Thomas Gleixner [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87seykl5im.ffs@tglx \
--to=tglx@linutronix.de \
--cc=a.hindborg@samsung.com \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=benno.lossin@proton.me \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=bp@alien8.de \
--cc=danilokrummrich@yahoo.de \
--cc=dave.hansen@linux.intel.com \
--cc=david@readahead.eu \
--cc=gary@garyguo.net \
--cc=hpa@zytor.com \
--cc=john.m.baublitz@gmail.com \
--cc=mingo@redhat.com \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=sergio.collado@gmail.com \
--cc=wedsonaf@gmail.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).