Re: [PATCH v2] config: avoid "write_in_full(fd, buf, len) < len" pattern

2017-09-15 Thread Ramsay Jones
On 15/09/17 01:37, Jeff King wrote: > On Thu, Sep 14, 2017 at 12:31:38AM +0100, Ramsay Jones wrote: > >> I just tried it again tonight; the current master branch has 3532 >> warnings when compiled with -Wextra, 1409 of which are -Wsign-compare >> warnings. After applying the patch below, those n

Re: [PATCH v2] config: avoid "write_in_full(fd, buf, len) < len" pattern

2017-09-14 Thread Jeff King
On Thu, Sep 14, 2017 at 12:31:38AM +0100, Ramsay Jones wrote: > > Hmm, about three or four years ago, I spent two or three evenings > > getting git to compile -Wextra clean. I remember the signed/unsigned > > issue was the cause of a large portion of the warnings issued by > > the compiler. I was

Re: [PATCH v2] config: avoid "write_in_full(fd, buf, len) < len" pattern

2017-09-13 Thread Ramsay Jones
On 13/09/17 23:43, Ramsay Jones wrote: > > > On 13/09/17 19:58, Jeff King wrote: >> On Wed, Sep 13, 2017 at 11:24:31AM -0700, Jonathan Nieder wrote: >> >>> For what it's worth, >>> Reviewed-by: Jonathan Nieder >> >> Thanks, and thank you for questioning the ptrdiff_t bits that didn't >> make s

Re: [PATCH v2] config: avoid "write_in_full(fd, buf, len) < len" pattern

2017-09-13 Thread Ramsay Jones
On 13/09/17 19:58, Jeff King wrote: > On Wed, Sep 13, 2017 at 11:24:31AM -0700, Jonathan Nieder wrote: > >> For what it's worth, >> Reviewed-by: Jonathan Nieder > > Thanks, and thank you for questioning the ptrdiff_t bits that didn't > make sense. I _thought_ they felt like nonsense, but could

Re: [PATCH v2] config: avoid "write_in_full(fd, buf, len) < len" pattern

2017-09-13 Thread Junio C Hamano
Jonathan Nieder writes: > Jeff King wrote: > >> What I missed is that copy_begin and copy_end here are actually size_t >> variables, not the pointers. Sorry for the confusion, and here's an >> updated version of the patch with this paragraph amended (the patch >> itself is identical): > > Subtle.

Re: [PATCH v2] config: avoid "write_in_full(fd, buf, len) < len" pattern

2017-09-13 Thread Jonathan Nieder
Jeff King wrote: > AFAIK there's no way to turn it on for specific functions, but I'm far > from a gcc-warning guru. Even if you could, though, the error may be far > from the function (e.g., if we store the result in an ssize_t and then > compare that with a size_t). It turns out that yes, we ha

Re: [PATCH v2] config: avoid "write_in_full(fd, buf, len) < len" pattern

2017-09-13 Thread Jonathan Nieder
Jeff King wrote: > On Wed, Sep 13, 2017 at 11:24:31AM -0700, Jonathan Nieder wrote: >> Compilers' signed/unsigned comparison warning can be noisy, but I'm >> starting to feel it's worth the suppression noise to turn it on when >> DEVELOPER=1 anyway. What do you think? Is there a way to turn it o

Re: [PATCH v2] config: avoid "write_in_full(fd, buf, len) < len" pattern

2017-09-13 Thread Jeff King
On Wed, Sep 13, 2017 at 11:24:31AM -0700, Jonathan Nieder wrote: > For what it's worth, > Reviewed-by: Jonathan Nieder Thanks, and thank you for questioning the ptrdiff_t bits that didn't make sense. I _thought_ they felt like nonsense, but couldn't quite puzzle it out. > Compilers' signed/unsi

Re: [PATCH v2] config: avoid "write_in_full(fd, buf, len) < len" pattern

2017-09-13 Thread Jonathan Nieder
Jeff King wrote: > What I missed is that copy_begin and copy_end here are actually size_t > variables, not the pointers. Sorry for the confusion, and here's an > updated version of the patch with this paragraph amended (the patch > itself is identical): Subtle. The world makes more sense now. T

[PATCH v2] config: avoid "write_in_full(fd, buf, len) < len" pattern

2017-09-13 Thread Jeff King
On Wed, Sep 13, 2017 at 01:11:04PM -0400, Jeff King wrote: > I scoured the code base for cases of this, but it turns out > that these two in git_config_set_multivar_in_file_gently() > are the only ones. This case is actually quite interesting: > we don't have a size_t, but rather use the subtracti