[PATCH] Fix application of sizeof to pointer

[PATCH] Fix application of sizeof to pointer

[davidcomponentone]

From: David Yang <davidcomponentone@gmail.com>

The coccinelle check report:
"./tools/testing/selftests/vm/split_huge_page_test.c:344:36-42:
ERROR: application of sizeof to pointer"
Using the "strlen" to fix it.

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: David Yang <davidcomponentone@gmail.com>
---
 tools/testing/selftests/vm/split_huge_page_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/vm/split_huge_page_test.c b/tools/testing/selftests/vm/split_huge_page_test.c
index 1af16d2c2a0a..52497b7b9f1d 100644
--- a/tools/testing/selftests/vm/split_huge_page_test.c
+++ b/tools/testing/selftests/vm/split_huge_page_test.c
@@ -341,7 +341,7 @@ void split_file_backed_thp(void)
 	}
 
 	/* write something to the file, so a file-backed THP can be allocated */
-	num_written = write(fd, tmpfs_loc, sizeof(tmpfs_loc));
+	num_written = write(fd, tmpfs_loc, strlen(tmpfs_loc) + 1);
 	close(fd);
 
 	if (num_written < 1) {
-- 
2.30.2

Re: [PATCH] Fix application of sizeof to pointer

[David Hildenbrand]

On 12.10.21 05:01, davidcomponentone@gmail.com wrote:
> From: David Yang <davidcomponentone@gmail.com>
> 
> The coccinelle check report:
> "./tools/testing/selftests/vm/split_huge_page_test.c:344:36-42:
> ERROR: application of sizeof to pointer"
> Using the "strlen" to fix it.
> 
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: David Yang <davidcomponentone@gmail.com>
> ---
>   tools/testing/selftests/vm/split_huge_page_test.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/vm/split_huge_page_test.c b/tools/testing/selftests/vm/split_huge_page_test.c
> index 1af16d2c2a0a..52497b7b9f1d 100644
> --- a/tools/testing/selftests/vm/split_huge_page_test.c
> +++ b/tools/testing/selftests/vm/split_huge_page_test.c
> @@ -341,7 +341,7 @@ void split_file_backed_thp(void)
>   	}
>   
>   	/* write something to the file, so a file-backed THP can be allocated */
> -	num_written = write(fd, tmpfs_loc, sizeof(tmpfs_loc));
> +	num_written = write(fd, tmpfs_loc, strlen(tmpfs_loc) + 1);
>   	close(fd);
>   
>   	if (num_written < 1) {
> 

I think the code really just wants to write anything to the file that 
will be >= 1, which is also the case with this weird usage of sizeof.

As an alternative, I think we can just write anything else to the file

num_written = write(fd, (void *)&fd, sizeof(fd));

or

num_written = write(fd, "1", 1);

If I am not wrong/

-- 
Thanks,

David / dhildenb

Re: [PATCH] Fix application of sizeof to pointer

[David Yang]

OK, Thanks.

On Wed, Oct 20, 2021 at 10:55:27AM -0700, Andrii Nakryiko wrote:
> On Tue, Oct 12, 2021 at 4:17 AM <davidcomponentone@gmail.com> wrote:
> >
> > From: David Yang <davidcomponentone@gmail.com>
> >
> > The coccinelle check report:
> > "./samples/bpf/xdp_redirect_cpu_user.c:397:32-38:
> > ERROR: application of sizeof to pointer"
> > Using the "strlen" to fix it.
> >
> > Reported-by: Zeal Robot <zealci@zte.com.cn>
> > Signed-off-by: David Yang <davidcomponentone@gmail.com>
> > ---
> 
> For future submissions, please use [PATCH bpf-next] subject prefix.
> For changes that are targeted against BPF samples, please use
> samples/bpf: prefix as well. So in this case the patch subject should
> have been something like:
> 
> [PATCH bpf-next] samples/bpf: Fix application of sizeof to pointer
> 
> I've fixed it up and applied to bpf-next, thanks.
> 
> >  samples/bpf/xdp_redirect_cpu_user.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
> > index 6e25fba64c72..d84e6949007c 100644
> > --- a/samples/bpf/xdp_redirect_cpu_user.c
> > +++ b/samples/bpf/xdp_redirect_cpu_user.c
> > @@ -325,7 +325,6 @@ int main(int argc, char **argv)
> >         int add_cpu = -1;
> >         int ifindex = -1;
> >         int *cpu, i, opt;
> > -       char *ifname;
> >         __u32 qsize;
> >         int n_cpus;
> >
> > @@ -393,9 +392,8 @@ int main(int argc, char **argv)
> >                                 fprintf(stderr, "-d/--dev name too long\n");
> >                                 goto end_cpu;
> >                         }
> > -                       ifname = (char *)&ifname_buf;
> > -                       safe_strncpy(ifname, optarg, sizeof(ifname));
> > -                       ifindex = if_nametoindex(ifname);
> > +                       safe_strncpy(ifname_buf, optarg, strlen(ifname_buf));
> > +                       ifindex = if_nametoindex(ifname_buf);
> >                         if (!ifindex)
> >                                 ifindex = strtoul(optarg, NULL, 0);
> >                         if (!ifindex) {
> > --
> > 2.30.2
> >

Re: [PATCH] Fix application of sizeof to pointer

[Andrii Nakryiko]

On Tue, Oct 12, 2021 at 4:17 AM <davidcomponentone@gmail.com> wrote:
>
> From: David Yang <davidcomponentone@gmail.com>
>
> The coccinelle check report:
> "./samples/bpf/xdp_redirect_cpu_user.c:397:32-38:
> ERROR: application of sizeof to pointer"
> Using the "strlen" to fix it.
>
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: David Yang <davidcomponentone@gmail.com>
> ---

For future submissions, please use [PATCH bpf-next] subject prefix.
For changes that are targeted against BPF samples, please use
samples/bpf: prefix as well. So in this case the patch subject should
have been something like:

[PATCH bpf-next] samples/bpf: Fix application of sizeof to pointer

I've fixed it up and applied to bpf-next, thanks.

>  samples/bpf/xdp_redirect_cpu_user.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
> index 6e25fba64c72..d84e6949007c 100644
> --- a/samples/bpf/xdp_redirect_cpu_user.c
> +++ b/samples/bpf/xdp_redirect_cpu_user.c
> @@ -325,7 +325,6 @@ int main(int argc, char **argv)
>         int add_cpu = -1;
>         int ifindex = -1;
>         int *cpu, i, opt;
> -       char *ifname;
>         __u32 qsize;
>         int n_cpus;
>
> @@ -393,9 +392,8 @@ int main(int argc, char **argv)
>                                 fprintf(stderr, "-d/--dev name too long\n");
>                                 goto end_cpu;
>                         }
> -                       ifname = (char *)&ifname_buf;
> -                       safe_strncpy(ifname, optarg, sizeof(ifname));
> -                       ifindex = if_nametoindex(ifname);
> +                       safe_strncpy(ifname_buf, optarg, strlen(ifname_buf));
> +                       ifindex = if_nametoindex(ifname_buf);
>                         if (!ifindex)
>                                 ifindex = strtoul(optarg, NULL, 0);
>                         if (!ifindex) {
> --
> 2.30.2
>

[PATCH] Fix application of sizeof to pointer

[davidcomponentone]

From: David Yang <davidcomponentone@gmail.com>

The coccinelle check report:
"./samples/bpf/xdp_redirect_cpu_user.c:397:32-38:
ERROR: application of sizeof to pointer"
Using the "strlen" to fix it.

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: David Yang <davidcomponentone@gmail.com>
---
 samples/bpf/xdp_redirect_cpu_user.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/samples/bpf/xdp_redirect_cpu_user.c b/samples/bpf/xdp_redirect_cpu_user.c
index 6e25fba64c72..d84e6949007c 100644
--- a/samples/bpf/xdp_redirect_cpu_user.c
+++ b/samples/bpf/xdp_redirect_cpu_user.c
@@ -325,7 +325,6 @@ int main(int argc, char **argv)
 	int add_cpu = -1;
 	int ifindex = -1;
 	int *cpu, i, opt;
-	char *ifname;
 	__u32 qsize;
 	int n_cpus;
 
@@ -393,9 +392,8 @@ int main(int argc, char **argv)
 				fprintf(stderr, "-d/--dev name too long\n");
 				goto end_cpu;
 			}
-			ifname = (char *)&ifname_buf;
-			safe_strncpy(ifname, optarg, sizeof(ifname));
-			ifindex = if_nametoindex(ifname);
+			safe_strncpy(ifname_buf, optarg, strlen(ifname_buf));
+			ifindex = if_nametoindex(ifname_buf);
 			if (!ifindex)
 				ifindex = strtoul(optarg, NULL, 0);
 			if (!ifindex) {
-- 
2.30.2

Re: [PATCH] Fix application of sizeof to pointer

[Viktor.Rosendahl]

On Mon, 2021-10-11 at 23:06 -0400, Steven Rostedt wrote:
> On Tue, 12 Oct 2021 10:54:24 +0800
> davidcomponentone@gmail.com wrote:
> 
> > From: David Yang <davidcomponentone@gmail.com>
> > 
> > The coccinelle check report:
> > "./tools/tracing/latency/latency-collector.c:1541:10-16:
> > ERROR: application of sizeof to pointer"
> > Using the "strlen" to fix it.
> > 
> > Reported-by: Zeal Robot <zealci@zte.com.cn>
> > Signed-off-by: David Yang <davidcomponentone@gmail.com>
> > ---
> >  tools/tracing/latency/latency-collector.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/tracing/latency/latency-collector.c
> > b/tools/tracing/latency/latency-collector.c
> > index 3a2e6bb781a8..b131007e6c70 100644
> > --- a/tools/tracing/latency/latency-collector.c
> > +++ b/tools/tracing/latency/latency-collector.c
> > @@ -1538,7 +1538,7 @@ static void tracing_loop(void)
> >  				mutex_lock(&print_mtx);
> >  				check_signals();
> >  				write_or_die(fd_stdout, queue_full_warning,
> > -					     sizeof(queue_full_warning));
> > +					     strlen(queue_full_warning) + 1);
> 
> Really, the +1, although would match if queue_full_warning was an
> array, but in this use case, there's no reason to pass the nul
> character to the write.

I agree that the +1 should not be there because the null character would end up
in the output; write will not ignore it. It could be confusing if the output
would be be processed by another program.

best regards,

Viktor

> 
> -- Steve
> 
> 
> >  				mutex_unlock(&print_mtx);
> >  			}
> >  			modified--;

Re: [PATCH] Fix application of sizeof to pointer

[Steven Rostedt]

On Tue, 12 Oct 2021 10:54:24 +0800
davidcomponentone@gmail.com wrote:

> From: David Yang <davidcomponentone@gmail.com>
> 
> The coccinelle check report:
> "./tools/tracing/latency/latency-collector.c:1541:10-16:
> ERROR: application of sizeof to pointer"
> Using the "strlen" to fix it.
> 
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: David Yang <davidcomponentone@gmail.com>
> ---
>  tools/tracing/latency/latency-collector.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/tracing/latency/latency-collector.c b/tools/tracing/latency/latency-collector.c
> index 3a2e6bb781a8..b131007e6c70 100644
> --- a/tools/tracing/latency/latency-collector.c
> +++ b/tools/tracing/latency/latency-collector.c
> @@ -1538,7 +1538,7 @@ static void tracing_loop(void)
>  				mutex_lock(&print_mtx);
>  				check_signals();
>  				write_or_die(fd_stdout, queue_full_warning,
> -					     sizeof(queue_full_warning));
> +					     strlen(queue_full_warning) + 1);

Really, the +1, although would match if queue_full_warning was an
array, but in this use case, there's no reason to pass the nul
character to the write.

-- Steve


>  				mutex_unlock(&print_mtx);
>  			}
>  			modified--;

[PATCH] Fix application of sizeof to pointer

[davidcomponentone]

From: David Yang <davidcomponentone@gmail.com>

The coccinelle check report:
"./tools/tracing/latency/latency-collector.c:1541:10-16:
ERROR: application of sizeof to pointer"
Using the "strlen" to fix it.

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: David Yang <davidcomponentone@gmail.com>
---
 tools/tracing/latency/latency-collector.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/tracing/latency/latency-collector.c b/tools/tracing/latency/latency-collector.c
index 3a2e6bb781a8..b131007e6c70 100644
--- a/tools/tracing/latency/latency-collector.c
+++ b/tools/tracing/latency/latency-collector.c
@@ -1538,7 +1538,7 @@ static void tracing_loop(void)
 				mutex_lock(&print_mtx);
 				check_signals();
 				write_or_die(fd_stdout, queue_full_warning,
-					     sizeof(queue_full_warning));
+					     strlen(queue_full_warning) + 1);
 				mutex_unlock(&print_mtx);
 			}
 			modified--;
-- 
2.30.2