Fix broken overflow check in posix_fallocate

  * sysdeps/posix/posix_fallocate.c (posix_fallocate):
* sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
Fix parenthesization typo.
---
 ChangeLog                         | 5 +++++
 sysdeps/posix/posix_fallocate.c   | 2 +-
 sysdeps/posix/posix_fallocate64.c | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)
  

On 08/26/2015 08:42 AM, Paul Eggert wrote:
> * sysdeps/posix/posix_fallocate.c (posix_fallocate):
> * sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
> Fix parenthesization typo.
> ---
>  ChangeLog                         | 5 +++++
>  sysdeps/posix/posix_fallocate.c   | 2 +-
>  sysdeps/posix/posix_fallocate64.c | 2 +-
>  3 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index bd6d027..458b850 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,10 @@
>  2015-08-25  Paul Eggert  <eggert@cs.ucla.edu>
>  
> +	Fix broken overflow check in posix_fallocate
> +	* sysdeps/posix/posix_fallocate.c (posix_fallocate):
> +	* sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
> +	Fix parenthesization typo.

Please add the BZ #18873 reference before committing (also to NEWS).

I don't recognize this changelog format.  Not sure if that's matching
the standards.

Otherwise okay, thanks.
  

On 08/26/2015 02:47 AM, Florian Weimer wrote:
> On 08/26/2015 08:42 AM, Paul Eggert wrote:
>> * sysdeps/posix/posix_fallocate.c (posix_fallocate):
>> * sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
>> Fix parenthesization typo.
>> ---
>>  ChangeLog                         | 5 +++++
>>  sysdeps/posix/posix_fallocate.c   | 2 +-
>>  sysdeps/posix/posix_fallocate64.c | 2 +-
>>  3 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/ChangeLog b/ChangeLog
>> index bd6d027..458b850 100644
>> --- a/ChangeLog
>> +++ b/ChangeLog
>> @@ -1,5 +1,10 @@
>>  2015-08-25  Paul Eggert  <eggert@cs.ucla.edu>
>>  
>> +	Fix broken overflow check in posix_fallocate
>> +	* sysdeps/posix/posix_fallocate.c (posix_fallocate):
>> +	* sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
>> +	Fix parenthesization typo.
> 
> Please add the BZ #18873 reference before committing (also to NEWS).
> 
> I don't recognize this changelog format.  Not sure if that's matching
> the standards.

In some GNU projects they encourage a general paragraph before the change
list to explain the fix. The GNU Coding Standard is silent about this
kind of additional text. We allow it, but I think Paul is the only one
who does this :-)

c.
  

Carlos O'Donell wrote:
> I think Paul is the only one
> who does this

It's standard in many other GNU projects (e.g., gnulib, coreutils), and fits 
well with the common Git style for commit messages.  GCC and GNU Emacs 
developers often use this style too, now, though not always.

It's a reasonable style and glibc should allow patches that use it, if only so 
that it's one less thing for occasional contributors to worry about.
  

On 08/28/2015 03:27 AM, Carlos O'Donell wrote:
> On 08/26/2015 02:47 AM, Florian Weimer wrote:
>> On 08/26/2015 08:42 AM, Paul Eggert wrote:
>>> * sysdeps/posix/posix_fallocate.c (posix_fallocate):
>>> * sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
>>> Fix parenthesization typo.
>>> ---
>>>  ChangeLog                         | 5 +++++
>>>  sysdeps/posix/posix_fallocate.c   | 2 +-
>>>  sysdeps/posix/posix_fallocate64.c | 2 +-
>>>  3 files changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/ChangeLog b/ChangeLog
>>> index bd6d027..458b850 100644
>>> --- a/ChangeLog
>>> +++ b/ChangeLog
>>> @@ -1,5 +1,10 @@
>>>  2015-08-25  Paul Eggert  <eggert@cs.ucla.edu>
>>>  
>>> +	Fix broken overflow check in posix_fallocate
>>> +	* sysdeps/posix/posix_fallocate.c (posix_fallocate):
>>> +	* sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
>>> +	Fix parenthesization typo.
>>
>> Please add the BZ #18873 reference before committing (also to NEWS).
>>
>> I don't recognize this changelog format.  Not sure if that's matching
>> the standards.
> 
> In some GNU projects they encourage a general paragraph before the change
> list to explain the fix. The GNU Coding Standard is silent about this
> kind of additional text. We allow it, but I think Paul is the only one
> who does this :-)

That part I had seen before, but I also expected something like this:

+	Fix broken overflow check in posix_fallocate
+	* sysdeps/posix/posix_fallocate.c (posix_fallocate):
+	Fix parenthesization typo.
+	* sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
+	Likewise.
  

On 08/28/2015 04:46 AM, Florian Weimer wrote:
> On 08/28/2015 03:27 AM, Carlos O'Donell wrote:
>> On 08/26/2015 02:47 AM, Florian Weimer wrote:
>>> On 08/26/2015 08:42 AM, Paul Eggert wrote:
>>>> * sysdeps/posix/posix_fallocate.c (posix_fallocate):
>>>> * sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
>>>> Fix parenthesization typo.
>>>> ---
>>>>  ChangeLog                         | 5 +++++
>>>>  sysdeps/posix/posix_fallocate.c   | 2 +-
>>>>  sysdeps/posix/posix_fallocate64.c | 2 +-
>>>>  3 files changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/ChangeLog b/ChangeLog
>>>> index bd6d027..458b850 100644
>>>> --- a/ChangeLog
>>>> +++ b/ChangeLog
>>>> @@ -1,5 +1,10 @@
>>>>  2015-08-25  Paul Eggert  <eggert@cs.ucla.edu>
>>>>  
>>>> +	Fix broken overflow check in posix_fallocate
>>>> +	* sysdeps/posix/posix_fallocate.c (posix_fallocate):
>>>> +	* sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
>>>> +	Fix parenthesization typo.
>>>
>>> Please add the BZ #18873 reference before committing (also to NEWS).
>>>
>>> I don't recognize this changelog format.  Not sure if that's matching
>>> the standards.
>>
>> In some GNU projects they encourage a general paragraph before the change
>> list to explain the fix. The GNU Coding Standard is silent about this
>> kind of additional text. We allow it, but I think Paul is the only one
>> who does this :-)
> 
> That part I had seen before, but I also expected something like this:
> 
> +	Fix broken overflow check in posix_fallocate
> +	* sysdeps/posix/posix_fallocate.c (posix_fallocate):
> +	Fix parenthesization typo.
> +	* sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
> +	Likewise.
 
Oh, I missed that. Agreed.

Cheers,
Carlos.
  

Florian Weimer wrote:
> That part I had seen before, but I also expected something like this:
>
> +	Fix broken overflow check in posix_fallocate
> +	* sysdeps/posix/posix_fallocate.c (posix_fallocate):
> +	Fix parenthesization typo.
> +	* sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
> +	Likewise.

Nowadays the "Likewise" style is considered old-fashioned in the other projects 
I hang out in.  Although it's fine and is still used on occasion, it feels a bit 
like writing "whereas" or "henceforth".  I don't know whether the newer style 
has been formally documented anywhere, but it's quite common and it is terser.
  

Two things about the recently-proposed bug fixes prompted by the Coverity scan.

First, Coverity itself does the scan for me. They use the glibc source to debug 
their scanner, and they send me their reports as a service to the community. I 
don't control the parameters they use. I found the latest set of four fixes by 
inspecting only the changes in reports from previous glibc. Most of the changes 
were false alarms (this is typical) and I generally left the code alone for the 
false alarms.

Second, there's a lot of bureaucracy involved in getting obvious bugs like these 
fixed. The current distracting thread about ChangeLog style is a symptom of the 
problem. In the old days I would have simply installed the fixes and moved on, 
but now I'm being asked for testcases and bugzilla references and ChangeLog 
style reformattting and whatnot and I don't want to upset anybody's applecart 
nor do I want to bother with all that paperwork (hey, I'm just a volunteer!) so 
I've done nothing to follow up, which is not good.

I can't help contrasting the somewhat offputting responses to my four glibc 
Coverity scan patches with the quite-friendly response I got from the gettext 
project when I filed the gettext-related patch upstream. Here's the entire 
interaction:

I ran "git send-email --to=bug-gettext@gnu.org".

Within a few hours, Daiki Ueno replied "Thanks, applied."

It's a pleasure helping to fix gettext bugs. I wish I could say the same thing 
about glibc.  I realize glibc is a bigger project and needs more bureaucracy 
yadda yadda yadda, but come on guys, we've gone off the deep end.

Anyway, to work through this little problem could you please install the patches 
for BZ#18868, BZ#18871, BZ#18872, and BZ#18873, whenever you have the time? The 
fixes themselves all have reasonable consensus, I think. Please feel free to dot 
all the Is and cross all the Ts that I missed. Thanks.
  

On 08/28/2015 11:19 AM, Paul Eggert wrote:
> Florian Weimer wrote:
>> That part I had seen before, but I also expected something like this:
>>
>> +    Fix broken overflow check in posix_fallocate
>> +    * sysdeps/posix/posix_fallocate.c (posix_fallocate):
>> +    Fix parenthesization typo.
>> +    * sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
>> +    Likewise.
> 
> Nowadays the "Likewise" style is considered old-fashioned in the
> other projects I hang out in. Although it's fine and is still used on
> occasion, it feels a bit like writing "whereas" or "henceforth". I
> don't know whether the newer style has been formally documented
> anywhere, but it's quite common and it is terser.

If you wouldn't mind, please stick to the existing project format for
this patch, but do start a new thread and ask:

* Does anyone mind if we support descriptive text in the ChangeLog?

* Does anyone mind if we use the terse "Likewise" format?

Then we'll get some yays and nays and update the Contribution Checklist.

I don't see why anyone would object, but I'd rather we talk about it,
briefly.

Cheers,
Carlos.
  

Carlos O'Donell wrote:
> * Does anyone mind if we use the terse "Likewise" format?

There must be some misunderstanding here, as the "Likewise" format is *less* 
terse than the format I used.  That is, I did this:

> +    Fix broken overflow check in posix_fallocate
> +    * sysdeps/posix/posix_fallocate.c (posix_fallocate):
> +    * sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
 > +    Fix parenthesization typo.

rather than this:

> +    Fix broken overflow check in posix_fallocate
> +    * sysdeps/posix/posix_fallocate.c (posix_fallocate):
> +    Fix parenthesization typo.
> +    * sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
> +    Likewise.
  

On 08/28/2015 01:52 PM, Paul Eggert wrote:
> Carlos O'Donell wrote:
>> * Does anyone mind if we use the terse "Likewise" format?
> 
> There must be some misunderstanding here, as the "Likewise" format is *less* terse than the format I used.  That is, I did this:
> 
>> +    Fix broken overflow check in posix_fallocate
>> +    * sysdeps/posix/posix_fallocate.c (posix_fallocate):
>> +    * sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
>> +    Fix parenthesization typo.
 
This is what I called the `terse "Likewise"` format.

> rather than this:
> 
>> +    Fix broken overflow check in posix_fallocate
>> +    * sysdeps/posix/posix_fallocate.c (posix_fallocate):
>> +    Fix parenthesization typo.
>> +    * sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
>> +    Likewise.

This is the standard one.

Does that clarify things?

c.
  

Carlos O'Donell wrote:
> Does that clarify things?

Yes.
  

On Fri, 28 Aug 2015, Carlos O'Donell wrote:

> * Does anyone mind if we support descriptive text in the ChangeLog?

Of course we should support it; it's permitted in the GNU Coding 
Standards.  I see no use in being stricter about ChangeLog format than 
what the GNU Coding Standards say (beyond the [BZ #N] annotations, where 
the GCS say nothing about how to indicate bug numbers).  (Of course, if we 
get automatic processing of commit messages to replace manual ChangeLog 
editing, we do need to get strict about formatting the commit message in 
the way expected by that automatic processing - but that's about enabling 
the processing to see what bit is the ChangeLog entry, not about the 
details of formatting within the ChangeLog entry.)

> * Does anyone mind if we use the terse "Likewise" format?

I'm happy with it.  That is, (a) with the format explicitly mentioned in 
the GCS:

	* file (func1, func2, func3)
	(func4, func5): Message.

and (b) with the generalization described where multiple files are 
involved.
  

On 08/26/2015 08:47 AM, Florian Weimer wrote:
> On 08/26/2015 08:42 AM, Paul Eggert wrote:
>> * sysdeps/posix/posix_fallocate.c (posix_fallocate):
>> * sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
>> Fix parenthesization typo.
>> ---
>>  ChangeLog                         | 5 +++++
>>  sysdeps/posix/posix_fallocate.c   | 2 +-
>>  sysdeps/posix/posix_fallocate64.c | 2 +-
>>  3 files changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/ChangeLog b/ChangeLog
>> index bd6d027..458b850 100644
>> --- a/ChangeLog
>> +++ b/ChangeLog
>> @@ -1,5 +1,10 @@
>>  2015-08-25  Paul Eggert  <eggert@cs.ucla.edu>
>>  
>> +	Fix broken overflow check in posix_fallocate
>> +	* sysdeps/posix/posix_fallocate.c (posix_fallocate):
>> +	* sysdeps/posix/posix_fallocate64.c (__posix_fallocate64_l64):
>> +	Fix parenthesization typo.
> 
> Please add the BZ #18873 reference before committing (also to NEWS).

I have committed this with the bug reference.
  

On 08/28/2015 05:49 PM, Paul Eggert wrote:

> Second, there's a lot of bureaucracy involved in getting obvious bugs
> like these fixed. The current distracting thread about ChangeLog style
> is a symptom of the problem.

I'm sorry you had a bad experience contributing.

The ChangeLog discussion was not entirely pointless.  I would not have
started it if the bug reference was not missing.  The posix_fallocate
bug would not have needed a bug report.

I'm not yet sure about the vfprintf bug.

> I can't help contrasting the somewhat offputting responses to my four
> glibc Coverity scan patches with the quite-friendly response I got from
> the gettext project when I filed the gettext-related patch upstream.
> Here's the entire interaction:
> 
> I ran "git send-email --to=bug-gettext@gnu.org".
> 
> Within a few hours, Daiki Ueno replied "Thanks, applied."

How can we import such patches in a way that the automated ChangeLog
merging kicks in?
  

Florian Weimer wrote:
> How can we import such patches in a way that the automated ChangeLog
> merging kicks in?

I haven't been following the details of the plans for glibc.  For Emacs, 
coreutils, etc., I use "git am" to import a patch from email.  There's no 
ChangeLog change to merge, since there's no ChangeLog under git control; the 
ChangeLog is computed automatically from the git commit log (which is part of 
the email).  I hope glibc will become just as easy to deal with.
  

On 08/31/2015 12:43 PM, Paul Eggert wrote:
> Florian Weimer wrote:
>> How can we import such patches in a way that the automated
>> ChangeLog merging kicks in?
> 
> I haven't been following the details of the plans for glibc.  For
> Emacs, coreutils, etc., I use "git am" to import a patch from email.
> There's no ChangeLog change to merge, since there's no ChangeLog
> under git control; the ChangeLog is computed automatically from the
> git commit log (which is part of the email).  I hope glibc will
> become just as easy to deal with.

We endeavour to get to the point where "git am" just works.

We had some rather long discussions about how to make progress on
this at GNU Cauldron 2015 in Prague.

c.