Re: clang-analyzer?
On 01/19/2017 09:07 PM, William A Rowe Jr wrote: > On Thu, Jan 19, 2017 at 1:30 PM, Ruediger Pluem wrote: >> >> If they are no-ops as you state in 3. how could they introduce regressions? > > They are still a text and code change. Cleaning up a cast, for example may > change the alignment differently between various 32 and 64 bit architectures. > Agreed. Sorry for nitpicking here, but than they are not no-ops like whitespace changes. They are low risk changes. Regards Rüdiger
Re: clang-analyzer?
On Thu, Jan 19, 2017 at 1:30 PM, Ruediger Pluem wrote: > > If they are no-ops as you state in 3. how could they introduce regressions? They are still a text and code change. Cleaning up a cast, for example may change the alignment differently between various 32 and 64 bit architectures.
Re: clang-analyzer?
On 01/19/2017 08:16 PM, William A Rowe Jr wrote: > On Mon, Jan 9, 2017 at 3:48 AM, Graham Leggett wrote: >> On 08 Jan 2017, at 4:45 AM, Leif Hedstrom wrote: >> >>> I ran clang-analyzer against the HTTPD master branch, and it found 126 >>> issues. Many of these are benign, but I was curious if the community has >>> any thoughts on this? With another project, I’ve found that keep static >>> code analysis to zero issues can really help finding new, serious issues >>> (basically, we put the tree in failed state if there’s a new static code >>> analysis issue). >>> >>> The issues are all over the source code, in core and mod_’s alike. It’d be >>> pretty tedious to file individual tickets for each of them, so curious if >>> there’s any interest in cleaning this up to start with a clean state? It’d >>> then be easy to add clang-analyzer to the release process for example. >> >> Adding clang-analyzer to a make target (not a default part of the build) >> would be a good step, it would make it easy for anyone to run it if they had >> it available. >> >> The most effective contributions would be patches to fix each one. From >> experience it is difficult to fix these sort of things without the ability >> to rerun the analyser to ensure the issue is gone, and every now and again >> issues uncover things that may take some time to fix. Agreed that getting >> these things to zero would be a good thing to have. > > Fixing those that are bugs is a no-brainer. > > Getting these to zero, let's say the 80% that don't represent bugs... > is a challenge. > > 1. We fix on trunk. Backports of later bug fixes are harder to apply. > > 2. We fix on trunk and backport to 2.4. Simplifies any later backports. >But this also introduces unnecessary regressions. If they are no-ops as you state in 3. how could they introduce regressions? > > My personal preference... > > 3. We create a patch to trunk for these issues (in a working branch >would seem most straightforward, but a git clone and fork could >also hold this work in progress.) Apply only once the beta to our >next major.minor appears to be acceptable for GA, and in one final >beta cycle, introduce these no-op changes, including whatever >formatting and whitespace cleanup is desired by the group. > > Third option also makes backports to the legacy branch harder to > apply, but at this point, fewer backports would be expected, so the > net merge time and hassle should be less than the first option of > committing no-op fixes to trunk as they are encountered. Regards Rüdiger
Re: clang-analyzer?
On Mon, Jan 9, 2017 at 3:48 AM, Graham Leggett wrote: > On 08 Jan 2017, at 4:45 AM, Leif Hedstrom wrote: > >> I ran clang-analyzer against the HTTPD master branch, and it found 126 >> issues. Many of these are benign, but I was curious if the community has any >> thoughts on this? With another project, I’ve found that keep static code >> analysis to zero issues can really help finding new, serious issues >> (basically, we put the tree in failed state if there’s a new static code >> analysis issue). >> >> The issues are all over the source code, in core and mod_’s alike. It’d be >> pretty tedious to file individual tickets for each of them, so curious if >> there’s any interest in cleaning this up to start with a clean state? It’d >> then be easy to add clang-analyzer to the release process for example. > > Adding clang-analyzer to a make target (not a default part of the build) > would be a good step, it would make it easy for anyone to run it if they had > it available. > > The most effective contributions would be patches to fix each one. From > experience it is difficult to fix these sort of things without the ability to > rerun the analyser to ensure the issue is gone, and every now and again > issues uncover things that may take some time to fix. Agreed that getting > these things to zero would be a good thing to have. Fixing those that are bugs is a no-brainer. Getting these to zero, let's say the 80% that don't represent bugs... is a challenge. 1. We fix on trunk. Backports of later bug fixes are harder to apply. 2. We fix on trunk and backport to 2.4. Simplifies any later backports. But this also introduces unnecessary regressions. My personal preference... 3. We create a patch to trunk for these issues (in a working branch would seem most straightforward, but a git clone and fork could also hold this work in progress.) Apply only once the beta to our next major.minor appears to be acceptable for GA, and in one final beta cycle, introduce these no-op changes, including whatever formatting and whitespace cleanup is desired by the group. Third option also makes backports to the legacy branch harder to apply, but at this point, fewer backports would be expected, so the net merge time and hassle should be less than the first option of committing no-op fixes to trunk as they are encountered.
Re: clang-analyzer?
> On Jan 9, 2017, at 4:48 AM, Graham Leggett wrote: > > On 08 Jan 2017, at 4:45 AM, Leif Hedstrom wrote: > >> I ran clang-analyzer against the HTTPD master branch, and it found 126 >> issues. Many of these are benign, but I was curious if the community has any >> thoughts on this? With another project, I’ve found that keep static code >> analysis to zero issues can really help finding new, serious issues >> (basically, we put the tree in failed state if there’s a new static code >> analysis issue). >> >> The issues are all over the source code, in core and mod_’s alike. It’d be >> pretty tedious to file individual tickets for each of them, so curious if >> there’s any interest in cleaning this up to start with a clean state? It’d >> then be easy to add clang-analyzer to the release process for example. > > Adding clang-analyzer to a make target (not a default part of the build) > would be a good step, it would make it easy for anyone to run it if they had > it available. > Sounds good to me.
Re: clang-analyzer?
On 01/09/2017 01:48 AM, Graham Leggett wrote: The most effective contributions would be patches to fix each one. From experience it is difficult to fix these sort of things without the ability to rerun the analyser to ensure the issue is gone, and every now and again issues uncover things that may take some time to fix. Agreed that getting these things to zero would be a good thing to have. +1 from me as well. --Jacob
Re: clang-analyzer?
On 1/9/2017 3:48 AM, Graham Leggett wrote: > Agreed that getting these things to zero would be a good thing to have. +1 here. The error report by itself is much less useful than an error report accompanied by a patch that addresses the benign things. -- Daniel Ruggeri
Re: clang-analyzer?
On 08 Jan 2017, at 4:45 AM, Leif Hedstrom wrote: > I ran clang-analyzer against the HTTPD master branch, and it found 126 > issues. Many of these are benign, but I was curious if the community has any > thoughts on this? With another project, I’ve found that keep static code > analysis to zero issues can really help finding new, serious issues > (basically, we put the tree in failed state if there’s a new static code > analysis issue). > > The issues are all over the source code, in core and mod_’s alike. It’d be > pretty tedious to file individual tickets for each of them, so curious if > there’s any interest in cleaning this up to start with a clean state? It’d > then be easy to add clang-analyzer to the release process for example. Adding clang-analyzer to a make target (not a default part of the build) would be a good step, it would make it easy for anyone to run it if they had it available. The most effective contributions would be patches to fix each one. From experience it is difficult to fix these sort of things without the ability to rerun the analyser to ensure the issue is gone, and every now and again issues uncover things that may take some time to fix. Agreed that getting these things to zero would be a good thing to have. Regards, Graham — smime.p7s Description: S/MIME cryptographic signature
Re: clang-analyzer?
Several times a year, we get offers or full dumps of programmatic static code analysis. We have, for decades, rejected it all, and invited reporters to bring specific analysis of actually problematic cases back to the list (or security@, as applicable.) If anyone is interested, we consistently invite reports of actual defects or security issues to be resolved. Cheers, Bill On Jan 7, 2017 8:45 PM, "Leif Hedstrom" wrote: > Howdy, > > I ran clang-analyzer against the HTTPD master branch, and it found 126 > issues. Many of these are benign, but I was curious if the community has > any thoughts on this? With another project, I’ve found that keep static > code analysis to zero issues can really help finding new, serious issues > (basically, we put the tree in failed state if there’s a new static code > analysis issue). > > The issues are all over the source code, in core and mod_’s alike. It’d be > pretty tedious to file individual tickets for each of them, so curious if > there’s any interest in cleaning this up to start with a clean state? It’d > then be easy to add clang-analyzer to the release process for example. > > Thoughts? > > — leif > >
