Il giorno lun 4 nov 2019 alle ore 16:11 Phillip Wood <phillip.wood123@xxxxxxxxx> ha scritto: > > Hi Elia > > On 04/11/2019 13:55, Elia Pinto wrote: > > Il giorno lun 4 nov 2019 alle ore 11:26 SZEDER Gábor > > <szeder.dev@xxxxxxxxx> ha scritto: > >> > >> On Mon, Nov 04, 2019 at 09:59:21AM +0000, Elia Pinto wrote: > >>> Fix the LGTM warning of the rule that finds uses of the assignment > >> > >> What is an "LGTM warning"? > >> > >> I think including the actual compiler warning in the commit message > >> would be great. > > Yes indeed. I thought I did it, do you think i can do better? Thanks > > > > https://lgtm.com/rules/2158670641/ > > It would have been helpful to explain that LGTM was a static analyser > for those of us who did not know. As far the patch is concerned I'm not > convinced it is an improvement. There are loads of places where git uses > this pattern ("git grep 'if (([^=)]*=[^)]*)' | wc" shows 212). So long > as the assignment is inside its own set of parentheses it's safe and gcc > will warn if the parentheses are missing. > LGTM only expresses a warning (actually it signals a possible error) that refers directly to this https://cwe.mitre.org/data/definitions/481.html. On which we can hardly disagree, beyond the authority of the source. The reason LGTM only identifies this as an error, and not the various apparently analogous cases in the code I don't know, but it would certainly be interesting to find out. Thanks for the review > Best Wishes > > Phillip > > >> > >>> operator = in places where the equality operator == would > >>> make more sense. > >>> > >>> Signed-off-by: Elia Pinto <gitter.spiros@xxxxxxxxx> > >>> --- > >>> ident.c | 13 ++++++++----- > >>> 1 file changed, 8 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/ident.c b/ident.c > >>> index e666ee4e59..07f2f03b0a 100644 > >>> --- a/ident.c > >>> +++ b/ident.c > >>> @@ -172,12 +172,15 @@ const char *ident_default_email(void) > >>> strbuf_addstr(&git_default_email, email); > >>> committer_ident_explicitly_given |= IDENT_MAIL_GIVEN; > >>> author_ident_explicitly_given |= IDENT_MAIL_GIVEN; > >>> - } else if ((email = query_user_email()) && email[0]) { > >>> - strbuf_addstr(&git_default_email, email); > >>> - free((char *)email); > >>> - } else > >>> - copy_email(xgetpwuid_self(&default_email_is_bogus), > >>> + } else { > >>> + email = query_user_email(); > >>> + if (email && email[0]) { > >>> + strbuf_addstr(&git_default_email, email); > >>> + free((char *)email); > >>> + } else > >>> + copy_email(xgetpwuid_self(&default_email_is_bogus), > >>> &git_default_email, &default_email_is_bogus); > >>> + } > >>> strbuf_trim(&git_default_email); > >>> } > >>> return git_default_email.buf; > >>> -- > >>> 2.24.0.rc0.467.g566ccdd3e4.dirty > >>>
