avoid useless assignment
PATCH 1/1 avoid useless assignment
--- src/openvpn/multi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
src/openvpn/multi.h +1 -1
@@ -633,7 +633,7 @@ multi_process_outgoing_tun(struct multi_context *m, const unsigned int mpp_flagsstatic inline bool multi_process_outgoing_link_dowork (struct multi_context *m,struct multi_instance *mi,const unsigned int mpp_flags) { -bool ret = true; +bool ret;set_prefix (mi);process_outgoing_link (&mi->context); ret =multi_process_post (m, mi, mpp_flags);
Signature · 1 line
2.14.1
My effort in writing the commit message has been quite poor.
The assignment is useless because 'ret' is re-assigned a few lines later without ever being read.
Cheers,
Quoted 19 lines · Antonio Quartulli
Signed-off-by Antonio Quartulli <[email protected]...>--- src/openvpn/multi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)src/openvpn/multi.h +1
-1@@ -633,7 +633,7 @@ multi_process_outgoing_tun(struct multi_context *m, const unsigned int mpp_flagsstatic inline bool multi_process_outgoing_link_dowork (struct multi_context *m,struct multi_instance *mi,const unsigned int mpp_flags) { -bool ret = true; +bool ret;set_prefix (mi);process_outgoing_link (&mi->context); ret =multi_process_post (m, mi, mpp_flags);
Signature · 1 line
Antonio Quartulli
Quoted 4 lines · Antonio Quartulli
My effort in writing the commit message has been quite poor.
The assignment is useless because 'ret' is re-assigned a few lines later without ever being read.
Hmmm. I'm not convinced of this change. But I'm also weird in these cases :)
I think it is good defensive programming to predefine the state of variables. When that is not done, it is up the the compiler to decide what to do - which most of the times does a sane job these days. But you're at the mercy of the compiler.
In this case, I would expect the compiler to optimize this out anyway, regardless of the approaches used. The compiler doesn't necessarily set the value first to true and then to change it to the output of multi_process_post(). It might just as well postpone the declaration.
So I think a better approach would be to completely move the "bool ret" down. So it will become:
bool ret = multi_process_post(m, mi, mpp_flags);
Which I think is also closer to what the compiler would end up with anyway.
Signature · 4 lines
kind regards, David Sommerseth OpenVPN Technologies, Inc
Quoted 20 lines · Antonio Quartulli
Quoted 19 lines
Signed-off-by Antonio Quartulli <[email protected]...>--- src/openvpn/multi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)src/openvpn/multi.h +1
-1@@ -633,7 +633,7 @@ multi_process_outgoing_tun(struct multi_context *m, const unsigned int mpp_flagsstatic inline bool multi_process_outgoing_link_dowork (struct multi_context *m,struct multi_instance *mi,const unsigned int mpp_flags) { -bool ret = true; +bool ret;set_prefix (mi);process_outgoing_link (&mi->context); ret =multi_process_post (m, mi, mpp_flags);
David Sommerseth <[email protected]...> on Thu, 2017/08/24 20:16:
Quoted 19 lines · Antonio Quartulli
Quoted 4 lines
My effort in writing the commit message has been quite poor.
The assignment is useless because 'ret' is re-assigned a few lines later without ever being read.
Hmmm. I'm not convinced of this change. But I'm also weird in these cases :)
I think it is good defensive programming to predefine the state of variables. When that is not done, it is up the the compiler to decide what to do - which most of the times does a sane job these days. But you're at the mercy of the compiler.
In this case, I would expect the compiler to optimize this out anyway, regardless of the approaches used. The compiler doesn't necessarily set the value first to true and then to change it to the output of
multi_process_post(). It might just as well postpone the declaration.
Quoted 6 lines
So I think a better approach would be to completely move the "bool ret" down. So it will become:
bool ret = multi_process_post(m, mi, mpp_flags);Which I think is also closer to what the compiler would end up with anyway.
ISO C90 forbids mixed declarations and code in C. Probably compilers will start to complain.
Signature · 3 lines
main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];)
putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}Quoted 23 lines · Christian Hesse
David Sommerseth <[email protected]...> on Thu, 2017/08/24 20:16:
Quoted 22 lines · Antonio Quartulli
Quoted 4 lines
My effort in writing the commit message has been quite poor.
The assignment is useless because 'ret' is re-assigned a few lines later without ever being read.
Hmmm. I'm not convinced of this change. But I'm also weird in these cases :)
I think it is good defensive programming to predefine the state of variables. When that is not done, it is up the the compiler to decide what to do - which most of the times does a sane job these days. But you're at the mercy of the compiler.
In this case, I would expect the compiler to optimize this out anyway, regardless of the approaches used. The compiler doesn't necessarily set the value first to true and then to change it to the output of
multi_process_post(). It might just as well postpone the declaration.So I think a better approach would be to completely move the "bool ret" down. So it will become:
Quoted 6 lines
Quoted 3 lines
bool ret = multi_process_post(m, mi, mpp_flags);Which I think is also closer to what the compiler would end up with anyway.
ISO C90 forbids mixed declarations and code in C. Probably compilers will start to complain.
We try to stick to C99. I think it allows such mix, no?
Cheers,
Signature · 1 line
Antonio Quartulli
Quoted 26 lines · Antonio Quartulli
Quoted 23 lines · Christian Hesse
David Sommerseth <[email protected]...> on Thu, 2017/08/24 20:16:
Quoted 22 lines · Antonio Quartulli
Quoted 4 lines
My effort in writing the commit message has been quite poor.
The assignment is useless because 'ret' is re-assigned a few lines later without ever being read.
Hmmm. I'm not convinced of this change. But I'm also weird in these cases :)
I think it is good defensive programming to predefine the state of variables. When that is not done, it is up the the compiler to decide what to do - which most of the times does a sane job these days. But you're at the mercy of the compiler.
In this case, I would expect the compiler to optimize this out anyway, regardless of the approaches used. The compiler doesn't necessarily set the value first to true and then to change it to the output of
multi_process_post(). It might just as well postpone the declaration.So I think a better approach would be to completely move the "bool ret" down. So it will become:
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
bool ret = multi_process_post(m, mi, mpp_flags);Which I think is also closer to what the compiler would end up with anyway.
ISO C90 forbids mixed declarations and code in C. Probably compilers will start to complain.
We try to stick to C99. I think it allows such mix, no?
That is correct. We set -std=c99 unless CFLAGS already contains -std=. But we expect OpenVPN to be C99 compliant.
And C99 allows this.
Signature · 4 lines
kind regards, David Sommerseth OpenVPN Technologies, Inc
David Sommerseth <[email protected]...> on Thu, 2017/08/24 20:51:
Quoted 36 lines · Antonio Quartulli
Quoted 35 lines · Christian Hesse
Quoted 31 lines
David Sommerseth <[email protected]...> on Thu, 2017/08/24 20:16:
Quoted 19 lines · Antonio Quartulli
Quoted 4 lines
My effort in writing the commit message has been quite poor.
The assignment is useless because 'ret' is re-assigned a few lines later without ever being read.
Hmmm. I'm not convinced of this change. But I'm also weird in these cases :)
I think it is good defensive programming to predefine the state of variables. When that is not done, it is up the the compiler to decide what to do - which most of the times does a sane job these days. But you're at the mercy of the compiler.
In this case, I would expect the compiler to optimize this out anyway, regardless of the approaches used. The compiler doesn't necessarily set the value first to true and then to change it to the output of
multi_process_post(). It might just as well postpone the declaration.Quoted 7 lines
So I think a better approach would be to completely move the "bool ret" down. So it will become:
bool ret = multi_process_post(m, mi, mpp_flags);Which I think is also closer to what the compiler would end up with anyway.
ISO C90 forbids mixed declarations and code in C. Probably compilers will start to complain.
Quoted 6 lines
Quoted 1 line
We try to stick to C99. I think it allows such mix, no?
That is correct. We set -std=c99 unless CFLAGS already contains -std=. But we expect OpenVPN to be C99 compliant.
And C99 allows this.
You are right... So scratch my concern.
Signature · 3 lines
main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/* Best regards my address: */=0;b=c[a++];)
putchar(b-1/(/* Chris cc -ox -xc - && ./x */b/42*2-3)*42);}