| Message ID | 20220805154231.31257-2-ojeda@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Rust support | expand |
On Fri, Aug 05, 2022 at 05:41:46PM +0200, Miguel Ojeda wrote: > From: Boqun Feng <boqun.feng@gmail.com> > > This removes one place where the `500` constant is hardcoded. > > Signed-off-by: Boqun Feng <boqun.feng@gmail.com> > Co-developed-by: Miguel Ojeda <ojeda@kernel.org> > Signed-off-by: Miguel Ojeda <ojeda@kernel.org> > --- > scripts/kallsyms.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c > index f18e6dfc68c5..52f5488c61bc 100644 > --- a/scripts/kallsyms.c > +++ b/scripts/kallsyms.c > @@ -206,7 +206,7 @@ static struct sym_entry *read_symbol(FILE *in) > > rc = fscanf(in, "%llx %c %499s\n", &addr, &type, name); > if (rc != 3) { > - if (rc != EOF && fgets(name, 500, in) == NULL) > + if (rc != EOF && fgets(name, sizeof(name), in) == NULL) > fprintf(stderr, "Read error or end of file.\n"); > return NULL; > } > -- > 2.37.1 > Signed-off-by: Geert Stappers <stappers@stappers.nl> And I think that this patch and all other "rust" kallsyms patches allready should have been accepted in the v3 or v5 series. Regards Geert Stappers
On Fri, Aug 5, 2022 at 6:48 PM Geert Stappers <stappers@stappers.nl> wrote: > > Signed-off-by: Geert Stappers <stappers@stappers.nl> Thanks for the message and the support, but please note that since you are not in the path of the patch, you cannot use this tag; instead look into Reviewed-by etc. See https://www.kernel.org/doc/html/v5.19/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by and the following section for details. > And I think that this patch and all other "rust" kallsyms patches > allready should have been accepted in the v3 or v5 series. Yeah, it could be a good idea to get the prerequisites in first. Let's see if the patches get some Reviewed-bys (e.g. I had to remove Kees' one because I had to split the patch). Cheers, Miguel
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c > index f18e6dfc68c5..52f5488c61bc 100644 > --- a/scripts/kallsyms.c > +++ b/scripts/kallsyms.c > @@ -206,7 +206,7 @@ static struct sym_entry *read_symbol(FILE *in) > > rc = fscanf(in, "%llx %c %499s\n", &addr, &type, name); > if (rc != 3) { > - if (rc != EOF && fgets(name, 500, in) == NULL) > + if (rc != EOF && fgets(name, sizeof(name), in) == NULL) > fprintf(stderr, "Read error or end of file.\n"); > return NULL; > } Might be another nit, but IMO it's better to use ARRAY_SIZE() here.
On Sat, Aug 06, 2022 at 01:40:33AM +0300, Konstantin Shelekhin wrote: > > diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c > > index f18e6dfc68c5..52f5488c61bc 100644 > > --- a/scripts/kallsyms.c > > +++ b/scripts/kallsyms.c > > @@ -206,7 +206,7 @@ static struct sym_entry *read_symbol(FILE *in) > > > > rc = fscanf(in, "%llx %c %499s\n", &addr, &type, name); > > if (rc != 3) { > > - if (rc != EOF && fgets(name, 500, in) == NULL) > > + if (rc != EOF && fgets(name, sizeof(name), in) == NULL) > > fprintf(stderr, "Read error or end of file.\n"); > > return NULL; > > } > > Might be another nit, but IMO it's better to use ARRAY_SIZE() here. I'm not sure I see a benefit for char arrays. It'll produce the same result, and the tradition for string functions is to use sizeof(). *shrug* Either way: Reviewed-by: Kees Cook <keescook@chromium.org>
On Wed, Aug 17, 2022 at 12:36:33PM -0700, Kees Cook wrote: > On Sat, Aug 06, 2022 at 01:40:33AM +0300, Konstantin Shelekhin wrote: > > > diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c > > > index f18e6dfc68c5..52f5488c61bc 100644 > > > --- a/scripts/kallsyms.c > > > +++ b/scripts/kallsyms.c > > > @@ -206,7 +206,7 @@ static struct sym_entry *read_symbol(FILE *in) > > > > > > rc = fscanf(in, "%llx %c %499s\n", &addr, &type, name); > > > if (rc != 3) { > > > - if (rc != EOF && fgets(name, 500, in) == NULL) > > > + if (rc != EOF && fgets(name, sizeof(name), in) == NULL) > > > fprintf(stderr, "Read error or end of file.\n"); > > > return NULL; > > > } > > > > Might be another nit, but IMO it's better to use ARRAY_SIZE() here. > > I'm not sure I see a benefit for char arrays. It'll produce the same > result, and the tradition for string functions is to use sizeof(). > *shrug* ARRAY_SIZE() (though not this one) can catch this: - char array[16]; + char *array; Saves me some.
On Thu, Aug 18, 2022 at 12:03:37PM +0300, Konstantin Shelekhin wrote: > On Wed, Aug 17, 2022 at 12:36:33PM -0700, Kees Cook wrote: > > On Sat, Aug 06, 2022 at 01:40:33AM +0300, Konstantin Shelekhin wrote: > > > > diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c > > > > index f18e6dfc68c5..52f5488c61bc 100644 > > > > --- a/scripts/kallsyms.c > > > > +++ b/scripts/kallsyms.c > > > > @@ -206,7 +206,7 @@ static struct sym_entry *read_symbol(FILE *in) > > > > > > > > rc = fscanf(in, "%llx %c %499s\n", &addr, &type, name); > > > > if (rc != 3) { > > > > - if (rc != EOF && fgets(name, 500, in) == NULL) > > > > + if (rc != EOF && fgets(name, sizeof(name), in) == NULL) > > > > fprintf(stderr, "Read error or end of file.\n"); > > > > return NULL; > > > > } > > > > > > Might be another nit, but IMO it's better to use ARRAY_SIZE() here. > > > > I'm not sure I see a benefit for char arrays. It'll produce the same > > result, and the tradition for string functions is to use sizeof(). > > *shrug* > > ARRAY_SIZE() (though not this one) can catch this: > > - char array[16]; > + char *array; > > Saves me some. Oh, that's an excellent point; I forgot it'll actually compile-time error if the var is a pointer. +1
On Thu, Aug 18, 2022 at 6:03 PM Kees Cook <keescook@chromium.org> wrote: > > Oh, that's an excellent point; I forgot it'll actually compile-time > error if the var is a pointer. +1 In this case it doesn't, because `scripts/kallsyms.c` defines its own `ARRAY_SIZE` that doesn't have the `__must_be_array`. I have changed it for v10 anyway, since that way we may benefit in the future if the `ARRAY_SIZE` here gains the check. Cheers, Miguel
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c index f18e6dfc68c5..52f5488c61bc 100644 --- a/scripts/kallsyms.c +++ b/scripts/kallsyms.c @@ -206,7 +206,7 @@ static struct sym_entry *read_symbol(FILE *in) rc = fscanf(in, "%llx %c %499s\n", &addr, &type, name); if (rc != 3) { - if (rc != EOF && fgets(name, 500, in) == NULL) + if (rc != EOF && fgets(name, sizeof(name), in) == NULL) fprintf(stderr, "Read error or end of file.\n"); return NULL; }