On 06/16/2015 07:06 AM, Pádraig Brady wrote:
> On 16/06/15 13:47, Eric Blake wrote:
>> [adding bug-gnulib]
>>
>> On 06/16/2015 06:28 AM, Johannes Meixner wrote:
>>>
>>>> From one of our (SUSE) kernel developers
>>> I even got a proposal for a workaround in grep:
>>>
>>> --- a/src/grep.c
>>> +++ b/src/grep.c
>>> @@ -575,6 +575,17 @@ file_textbin (char *buf, size_t size, in
>>> off_t hole_start = lseek (fd, cur, SEEK_HOLE);
>>> if (0 <= hole_start)
>>> {
>>> + /* if hole_start is identical with cur, it's more likely a
>>> buggy
>>> + * lseek(SEEK_HOLE) implementation in kernel filesystem;
>>> + * check whether it really reads a null byte.
>>> + */
>>> + if (hole_start == cur)
>>> + {
>>> + char c;
>>> + if (read (fd, &c, 1) != 1 || c != 0)
>>> + hole_start = st->st_size; /* no hole */You MUST lseek() back to the original spot (or use pread() to avoid changing offset) for the workaround to work. There might also be ways to cache whether the workaround has ever been needed, so that subsequent calls aren't penalized (then again, when working around the bug, the likely behavior is to just treat the file as all data, so you'll only ever encounter the bug once per file, unless you are reading the file's contents multiple times); and as the call is filesystem related, we can't guarantee that it works or fails on all possible files served by the kernel, when crossing device boundaries. So yeah, we're probably stuck with a per-file probing, minimized to the cases where a reported hole sounds suspicious. >>> + } >>> + >> >> Sounds like we need to update the gnulib lseek module to work around >> bugs like this in the wild. > > Interesting. The above runtime check wouldn't be appropriate I think > for such an old fleeting problem. Also the read() as a side effect > would be very much best avoided. How about a linux kernel version check > at build time? > > As a general point, gnulib would really benefit from SEEK_HOLE, SEEK_DATA > wrappers that degenerated to SEEK_END,0 and SEEK_SET,offset respectively. Yeah, I've wanted those for several years now (I even mentioned it as a "nice to have" in my Linux Plumber's Convention talk in 2012), but haven't had time to work on it myself yet. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
