[PATCH v8 02/31] kallsyms: avoid hardcoding buffer size

Miguel Ojeda posted 31 patches 3 years, 8 months ago
There is a newer version of this series
[PATCH v8 02/31] kallsyms: avoid hardcoding buffer size
Posted by Miguel Ojeda 3 years, 8 months ago
From: Boqun Feng <boqun.feng@gmail.com>

This introduces `KSYM_NAME_LEN_BUFFER` in place of the previously
hardcoded size of the input buffer.

It will also make it easier to update the size in a single place
in a later patch.

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 | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 52f5488c61bc..f3c5a2623f71 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -27,8 +27,14 @@
 
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof(arr[0]))
 
+#define _stringify_1(x)	#x
+#define _stringify(x)	_stringify_1(x)
+
 #define KSYM_NAME_LEN		128
 
+/* A substantially bigger size than the current maximum. */
+#define KSYM_NAME_LEN_BUFFER	499
+
 struct sym_entry {
 	unsigned long long addr;
 	unsigned int len;
@@ -198,13 +204,13 @@ static void check_symbol_range(const char *sym, unsigned long long addr,
 
 static struct sym_entry *read_symbol(FILE *in)
 {
-	char name[500], type;
+	char name[KSYM_NAME_LEN_BUFFER+1], type;
 	unsigned long long addr;
 	unsigned int len;
 	struct sym_entry *sym;
 	int rc;
 
-	rc = fscanf(in, "%llx %c %499s\n", &addr, &type, name);
+	rc = fscanf(in, "%llx %c %" _stringify(KSYM_NAME_LEN_BUFFER) "s\n", &addr, &type, name);
 	if (rc != 3) {
 		if (rc != EOF && fgets(name, sizeof(name), in) == NULL)
 			fprintf(stderr, "Read error or end of file.\n");
-- 
2.37.1
RE: [PATCH v8 02/31] kallsyms: avoid hardcoding buffer size
Posted by David Laight 3 years, 8 months ago
From: Miguel Ojeda
> Sent: 02 August 2022 02:50
> 
> From: Boqun Feng <boqun.feng@gmail.com>
> 
> This introduces `KSYM_NAME_LEN_BUFFER` in place of the previously
> hardcoded size of the input buffer.
> 
> It will also make it easier to update the size in a single place
> in a later patch.
> 
> 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 | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index 52f5488c61bc..f3c5a2623f71 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -27,8 +27,14 @@
> 
>  #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof(arr[0]))
> 
> +#define _stringify_1(x)	#x
> +#define _stringify(x)	_stringify_1(x)
> +
>  #define KSYM_NAME_LEN		128
> 
> +/* A substantially bigger size than the current maximum. */
> +#define KSYM_NAME_LEN_BUFFER	499
> +
>  struct sym_entry {
>  	unsigned long long addr;
>  	unsigned int len;
> @@ -198,13 +204,13 @@ static void check_symbol_range(const char *sym, unsigned long long addr,
> 
>  static struct sym_entry *read_symbol(FILE *in)
>  {
> -	char name[500], type;
> +	char name[KSYM_NAME_LEN_BUFFER+1], type;
>  	unsigned long long addr;
>  	unsigned int len;
>  	struct sym_entry *sym;
>  	int rc;
> 
> -	rc = fscanf(in, "%llx %c %499s\n", &addr, &type, name);
> +	rc = fscanf(in, "%llx %c %" _stringify(KSYM_NAME_LEN_BUFFER) "s\n", &addr, &type, name);

Think I'd use "%*s" - simpler.
Although I normally completely avoid scanf() - too easy to get wrong.

	David

>  	if (rc != 3) {
>  		if (rc != EOF && fgets(name, sizeof(name), in) == NULL)
>  			fprintf(stderr, "Read error or end of file.\n");
> --
> 2.37.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Re: [PATCH v8 02/31] kallsyms: avoid hardcoding buffer size
Posted by Rasmus Villemoes 3 years, 8 months ago
On 02/08/2022 10.29, David Laight wrote:
> From: Miguel Ojeda
>> Sent: 02 August 2022 02:50

>> -	rc = fscanf(in, "%llx %c %499s\n", &addr, &type, name);
>> +	rc = fscanf(in, "%llx %c %" _stringify(KSYM_NAME_LEN_BUFFER) "s\n", &addr, &type, name);
> 
> Think I'd use "%*s" - simpler.
> Although I normally completely avoid scanf() - too easy to get wrong.

Indeed, and your suggestion is a perfect example: for scanf, * doesn't
mean "there's an int argument specifying the 'precision'", quite the
contrary. man fscanf:

   •      An  optional  '*'  assignment-suppression  character:  scanf()
          reads  input  as directed by the conversion specification, but
          discards the input.  No corresponding pointer argument is  re‐
          quired, and this specification is not included in the count of
          successful assignments returned by scanf().

Rasmus