| Message ID | 25ea92f064e11ba30ae696b176df9d6b0aaaa66a.1628352013.git.aclaudi@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Stephen Hemminger |
| Headers | show |
| Series | [iproute2] lib: bpf_glue: remove useless assignment | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Sat, 7 Aug 2021 18:58:02 +0200 Andrea Claudi <aclaudi@redhat.com> wrote: > - while ((s = fgets(buf, sizeof(buf), fp)) != NULL) { > + while (fgets(buf, sizeof(buf), fp) != NULL) { > if ((s = strstr(buf, "libbpf.so.")) != NULL) { Ok. but it would be good to get rid of the unnecessary assignment in conditional as well.
On Tue, Aug 10, 2021 at 08:00:48PM -0700, Stephen Hemminger wrote: > On Sat, 7 Aug 2021 18:58:02 +0200 > Andrea Claudi <aclaudi@redhat.com> wrote: > > > - while ((s = fgets(buf, sizeof(buf), fp)) != NULL) { > > + while (fgets(buf, sizeof(buf), fp) != NULL) { > > if ((s = strstr(buf, "libbpf.so.")) != NULL) { > > Ok. but it would be good to get rid of the unnecessary assignment in conditional as well. > Hi Stephen, That's not unnecessary, s is used as the second parameter in the following strncpy().
On Wed, 11 Aug 2021 11:12:43 +0200 Andrea Claudi <aclaudi@redhat.com> wrote: > On Tue, Aug 10, 2021 at 08:00:48PM -0700, Stephen Hemminger wrote: > > On Sat, 7 Aug 2021 18:58:02 +0200 > > Andrea Claudi <aclaudi@redhat.com> wrote: > > > > > - while ((s = fgets(buf, sizeof(buf), fp)) != NULL) { > > > + while (fgets(buf, sizeof(buf), fp) != NULL) { > > > if ((s = strstr(buf, "libbpf.so.")) != NULL) { > > > > Ok. but it would be good to get rid of the unnecessary assignment in conditional as well. > > > Hi Stephen, > That's not unnecessary, s is used as the second parameter in the following strncpy(). > It is bad style in C to do assignment in a conditional. It causes errors, and is not anymore efficient.
On Wed, Aug 11, 2021 at 09:08:15AM -0700, Stephen Hemminger wrote: > It is bad style in C to do assignment in a conditional. > It causes errors, and is not anymore efficient. > I agree with you. There is a large number of similar assignments in other parts of the code; I can work on a treewide patch to address them all, if you think it's a good idea.
On Thu, 12 Aug 2021 11:01:42 +0200 Andrea Claudi <aclaudi@redhat.com> wrote: > On Wed, Aug 11, 2021 at 09:08:15AM -0700, Stephen Hemminger wrote: > > It is bad style in C to do assignment in a conditional. > > It causes errors, and is not anymore efficient. > > > I agree with you. > > There is a large number of similar assignments in other parts of the > code; I can work on a treewide patch to address them all, if you think > it's a good idea. > I am looking into this, checkpatch seems to find them
diff --git a/lib/bpf_glue.c b/lib/bpf_glue.c index eaa9504f..70d00184 100644 --- a/lib/bpf_glue.c +++ b/lib/bpf_glue.c @@ -63,7 +63,7 @@ const char *get_libbpf_version(void) if (fp == NULL) goto out; - while ((s = fgets(buf, sizeof(buf), fp)) != NULL) { + while (fgets(buf, sizeof(buf), fp) != NULL) { if ((s = strstr(buf, "libbpf.so.")) != NULL) { strncpy(_libbpf_version, s+10, sizeof(_libbpf_version)-1); strtok(_libbpf_version, "\n");
The value of s used inside the cycle is the result of strstr(), so this assignment is useless. Signed-off-by: Andrea Claudi <aclaudi@redhat.com> --- lib/bpf_glue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)