| Message ID | 55E14093.1070407@charter.net |
|---|---|
| State | New |
| Headers | show |
Hi Jerry, The patch is OK, but I’m a bit puzzled about what the testcase does. It tests that we can OPEN a directory, but not READ from it? I didn’t know that was expected (to be able to OPEN a directory), and I find it somewhat puzzling. Can you shed light on that? Thanks, FX
On 08/28/2015 11:59 PM, FX wrote: > Hi Jerry, > > The patch is OK, but I’m a bit puzzled about what the testcase does. > > It tests that we can OPEN a directory, but not READ from it? I didn’t know that was expected (to be able to OPEN a directory), and I find it somewhat puzzling. Can you shed light on that? > > Thanks, > FX > According to my man pages, open is used to open a directory as read only and readdir must be used to actually read it. It is not allowed to open a directory with write access. The test case is checking that the open works as expected and the attempt to read using 'read' results in the error (no segfault) Jerry PS thanks for the review.
On 08/28/2015 10:18 PM, Jerry DeLisle wrote: > I found that in read_buf where raw_read is called, no checks for errors were > being made, raw_read returns the number of bytes read or an error code. In the > test case, an error occurs and we proceeded to use the resulting error code as > if it were the number of bytes read. > > The attached patch fixes this. Regression tested on x86_64. New test case > provided. > > OK for trunk? > Committed. I should mention, the test case may fail on some platforms since the system call may be dependent. I will xfail those as we learn which. Test case committed revision 227321. Regards, Jerry
Index: unix.c =================================================================== --- unix.c (revision 227314) +++ unix.c (working copy) @@ -529,16 +529,26 @@ buf_read (unix_stream * s, void * buf, ssize_t nby if (to_read <= BUFFER_SIZE/2) { did_read = raw_read (s, s->buffer, BUFFER_SIZE); - s->physical_offset += did_read; - s->active = did_read; - did_read = (did_read > to_read) ? to_read : did_read; - memcpy (p, s->buffer, did_read); + if (likely (did_read >= 0)) + { + s->physical_offset += did_read; + s->active = did_read; + did_read = (did_read > to_read) ? to_read : did_read; + memcpy (p, s->buffer, did_read); + } + else + return did_read; } else { did_read = raw_read (s, p, to_read); - s->physical_offset += did_read; - s->active = 0; + if (likely (did_read >= 0)) + { + s->physical_offset += did_read; + s->active = 0; + } + else + return did_read; } nbyte = did_read + nread; }