some lintings

[fperrad]

No description provided.

[sjaeckel]

is there a reason why we shouldn't return err and instead return a generic CRYPT_ERROR?

[fperrad]

No reason, and it is the usual way of doing in others files.
I'll refactor this patch and rename it "use the variable 'err'".

[sjaeckel]

Is there a way for us to reproduce these changes? especially all the formatting stuff...

[karel-m]

I went through changes in this PR and do not see anything I would object.

Considering the huge number of changes I propose in the first step to cherrypick to develop only the commits dealing with white spaces and comments and then see what will be left here.

I mean:

fix indentation                                             0bb494c
the comment FALLTHROUGH is common for several lint tool     ce5c27e
remove hard tab                                             5e90b62
remove trailing spaces                                      605b4d6

[sjaeckel]

I'd add into the list of no-brainers:

remove useless include					598533b
add some const						3ed59f4
remove useless code					6e7c109

and these into the list of important fixes:

add parenthese in macro					3d2efa3
bug: wrong parentheses in condition with assignment	25558cf
use the variable 'err'					c150fe9

[karel-m]

the first part of the changes merged to develop

fix indentation                                         5148852
remove useless include                                  affe44f
the comment FALLTHROUGH is common for several lint tool e61caf3
add some const                                          5f79489
remove useless code                                     49b3c92
remove hard tab                                         75e0949
remove trailing spaces                                  5507fda

@fperrad could you please rebase the remaining commits on current develop?

[karel-m]

The remaining part:

default for switch                                      563aea5
Suspicious use of ;                                     cad5bc5
Suspicious use of &                                     297e3ff
add parenthese in macro                                 01effe9
bug: wrong parentheses in condition with assignment     66f0c33
add parentheses for explicit operator association       a021412
use the variable 'err'                                  df96a18

[karel-m]

The next group I propose to accept without changes is:

default for switch                                      563aea5
use the variable 'err'                                  df96a18
bug: wrong parentheses in condition with assignment     66f0c33
add parenthese in macro                                 01effe9

@sjaeckel agree?

[karel-m]

@fperrad could you please split Suspicious use of & into two commits

1/ part changing pointers to a functions (vast majority)

2/ part changing other pointers

While 1/ is IMO just a question of personal taste the 2/ might be bug(s)

[fperrad]

@karel-m done

[karel-m]

So the second set merged to develop.

@fperrad please do rebase once again

There should remain the following:

Suspicious use of & (part 1)                            b408017
Suspicious use of & (part 2)                            7a3f8ed
Suspicious use of ;                                     de80ff3
add parentheses for explicit operator association       0e60362

[karel-m]

@fperrad could you please also move qsort_helper patches (2 places: der_encode_set.c + der_encode_setof.c) from Suspicious use of & (part 2) to part 1?

[karel-m]

Here are my objections:

Ad add parentheses for explicit operator association

• I do not consider it as an improvement
• all cases are clear enough or even clearer in the original code
• I am for rejecting this commit 👎

Ad Suspicious use of & (part 1)

• it is equivalent to use &func_name and func_name
• my personal preference is to use &func_name therefore not accepting this commit
• but if it gains some other supporters I can live with that

Ad Suspicious use of ;

• I believe that empty C loops like for(....); are not that unusual
• I am not a fan of for(....) {} form, but if it gains some other supporters I can live with that

[karel-m]

OK, now I vote 👍 for applying Suspicious use of & (part 2) 4a50707

@sjaeckel ?

[sjaeckel]

Ad add parentheses for explicit operator association

@fperrad what's the rational behind this patch?

In general I also think that it's less clear and I would also tend to reject the patch, but probably there's an objective reason that I'm not aware of yet.

Ad Suspicious use of & (part 1) & Suspicious use of ;

I've no opinion on these patches, would also tend to not apply but I've no problem applying them if there's a real reasoning behind.

[karel-m]

Suspicious use of & (part 2) is now merged into develop

[karel-m]

@fperrad please move static functions to a separate PR

[fperrad]

for a rational of Suspicious use of & (part 1) see http://www.gimpel-online.com/MsgRef.html#546

I also found some discussions in https://stackoverflow.com/questions/9552663/function-pointers-and-address-of-a-function

[coveralls]

Coverage remained the same at 87.275% when pulling 7c4cdb0 on fperrad:20151219_lint into 8141ca6 on libtom:develop.

[karel-m]

I have check the links above but not changed my mind.

As there are no other supporters I propose to close this PR and not to merge the remaining "Suspicious use of .." commits.

[karel-m]

Closing as suggested 2 weeks ago.