* Re: [PATCH] Add room for insn-enconding and symbol name
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 16:31 ` Thomas Gleixner
1 sibling, 1 reply; 5+ messages in thread
From: Danilo Krummrich @ 2024-05-08 11:58 UTC (permalink / raw)
To: Sergio González Collado, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H . Peter Anvin, Miguel Ojeda,
Alex Gaynor, Wedson Almeida Filho
Cc: David Rheinsberg, John Baublitz, x86, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
rust-for-linux
On 4/14/24 20:09, Sergio González Collado wrote:
> 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]
>
> [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>
Acked-by: Danilo Krummrich <dakr@redhat.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>
>
> #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)
>
> int main(int argc, char **argv)
> {
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] Add room for insn-enconding and symbol name
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 16:31 ` Thomas Gleixner
1 sibling, 0 replies; 5+ messages in thread
From: Thomas Gleixner @ 2024-05-14 16:31 UTC (permalink / raw)
To: Sergio González Collado, Ingo Molnar, Borislav Petkov,
Dave Hansen, H . Peter Anvin, Miguel Ojeda, Alex Gaynor,
Wedson Almeida Filho
Cc: David Rheinsberg, John Baublitz, Danilo Krummrich, x86,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, rust-for-linux,
Sergio González Collado
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
^ permalink raw reply [flat|nested] 5+ messages in thread