Re: [Bug-readline] [PATCH] readline/histfile.c: Check and retry write() operation in history_truncate_file()

[Chen Gang <gang dot chen dot 5i5j at gmail dot com>]

On 06/21/2014 05:53 AM, Chet Ramey wrote:
> On 6/20/14, 4:57 AM, Chen Gang wrote:
>> On 06/19/2014 09:31 AM, Chen Gang wrote:
>>>
>>>
>>> On 06/19/2014 04:33 AM, Chet Ramey wrote:
>>>> On 6/10/14, 10:35 PM, Chen Gang wrote:
>>>>> For regular file, write() operation may also fail, so check it too. If
>>>>> write() return 0, can simply wait and try again, it should not suspend
>>>>> infinitely if environments have no critical issues.
>>>>
>>>> Readline-6.3 checks the return value from write() and returns a non-zero
>>>> value to the history_truncate_file caller.  I really don't think that
>>>> waiting forever if write continues to return 0 is a great idea; an error
>>>> return is enough to let the caller deal with it.
>>>>
>>
>> Oh, sorry, after think of again, for me, we have to waiting forever if
>> write() continues to return 0.
> 
> There aren't really any plausible conditions under which write(2) returns
> 0 instead of -1 when writing a non-zero number of bytes to a regular file.
> 

Hmm... for me, what you said is acceptable.  And the function comment need be
changed:

  "Returns 0 on success, errno or -1 on failure"

>>
>> When this case happens, the file is already truncated, and the left data
>> which is writing to file will be free after return from
>> history_truncate_file().
>>
>> If return an error code in this case, the caller can not deal with it --
>> the log data which should be remained, have been lost, can not get them
>> back again.
> 
> However, you're right about the data being lost if write fails and returns
> -1. Since the sequence of operations is open-read-close-open-write-close, I
> think I will change the code for the next version to use a scheme similar
> to history_do_write() and restore the original version of the file if the
> write fails.
> 

OK, thanks.

And since you will work for it next, is it still necessary to send patch v2 for
the temporary fix, at present?


Thanks
-- 
Open share and attitude like air warter and life which God blessed