Re: [PATCH] Handle short reads and EINTR in lto-plugin/simple-object

[Richard Biener <rguenther at suse dot de>]

On March 26, 2014 4:51:58 PM CET, Ian Lance Taylor <iant@google.com> wrote:
>On Wed, Mar 26, 2014 at 8:38 AM, Richard Biener <rguenther@suse.de>
>wrote:
>>
>> -  got = read (descriptor, buffer, size);
>> -  if (got < 0)
>> +  do
>>      {
>> -      *errmsg = "read";
>> -      *err = errno;
>> -      return 0;
>> +      got = read (descriptor, buffer, size);
>> +      if (got < 0
>> +         && errno != EINTR)
>> +       {
>> +         *errmsg = "read";
>> +         *err = errno;
>> +         return 0;
>> +       }
>> +      else
>> +       {
>> +         buffer += got;
>> +         size -= got;
>> +       }
>
>This appears to do the wrong thing if got < 0 && errno == EINTR.  In
>that case it should not add got to buffer and size.

Uh, indeed. Will fix.

>> -  if (offset != lseek (obj->file->fd, offset, SEEK_SET)
>> -       || length != read (obj->file->fd, secdata, length))
>> +  if (!simple_object_internal_read (obj->file->fd, offset,
>> +                                   secdata, length, &errmsg, &err))
>
>Hmmm, internal_read is meant to be, well, internal.  It's not declared
>anywhere as far as I can see.

I can duplicate the stuff as well.

>Are you really seeing EINTR reads here?  That seems very odd to me,
>since we are always just reading a local file.  But if you are seeing
>it, I guess we should handle it.

Well, it's a shot in the dark... I definitely know short reads and EINTR happens more in virtual machines though. So handling it is an improvement.

I'll see if it fixes my problems and report back.

Thanks,
Richard.

>Ian