[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-gnulib] full-write.c / full-read.c consolidation
From: |
Jim Meyering |
Subject: |
Re: [Bug-gnulib] full-write.c / full-read.c consolidation |
Date: |
Wed, 08 Jan 2003 17:53:42 +0100 |
Bruno Haible <address@hidden> wrote:
> Jim Meyering writes:
>> Any objection to this?
>>
>> * full-write.c: Rework so that it may serve to define full_read, too.
>> * full-read.c: Simply #define FULL_READ and include full-write.c.
>>
>> With the above change, full-read.c is just two lines (modulo copyright):
>
> Nothing against unification of duplicated code - where it is
> duplicated and not only similar. But here, when 50% of the resulting
> code is #ifdef'ed, my gut feeling is that we are going too far. The
> #ifdef spaghetti is harder to maintain than the two earlier functions.
Hi Bruno,
I didn't like the in-function #ifdef either, so factored that out
after sending that message but before checking in to gnulib.
The new code is cleaner. Here it is:
size_t
full_rw (int fd, const void *buf, size_t count)
{
size_t total = 0;
const char *ptr = buf;
while (count > 0)
{
size_t n_rw = safe_rw (fd, ptr, count);
if (n_rw == (size_t) -1)
break;
if (n_rw == 0)
{
errno = ZERO_BYTE_TRANSFER_ERRNO;
break;
}
total += n_rw;
ptr += n_rw;
count -= n_rw;
}
return total;
}
I don't like all the #ifdef'd includes.
They're just for prototypes after all, so it shouldn't
hurt to include e.g. safe-read.h from safe-write.c.
>> # undef const
>> # define const /* empty */
Thanks for catching that.
I've just moved the #include `up'.
> I think it would be safer to do this after including <errno.h>, not
> before. You never know what <errno.h> contains.
>
>> const char *ptr = buf;
>>
>> while (count > 0)
>
> Any reason why you undid the "really nothing to do if count == 0"
> optimization that I had put in?
I found that it made the code much less readable, and for
probably-imperceptible gain. Besides, who cares how efficient the
code is when it's asked to do nothing? Of course, if you know of some
application where that single unnecessary store (*ptr = buf;) actually
makes a difference, then I'll be happy to reconsider.
Jim
- [Bug-gnulib] full-write.c / full-read.c consolidation, Jim Meyering, 2003/01/07
- Re: [Bug-gnulib] full-write.c / full-read.c consolidation, Karl Berry, 2003/01/07
- Re: [Bug-gnulib] full-write.c / full-read.c consolidation, Karl Berry, 2003/01/08
- Re: [Bug-gnulib] full-write.c / full-read.c consolidation, Karl Berry, 2003/01/08
- Re: [Bug-gnulib] full-write.c / full-read.c consolidation, Karl Berry, 2003/01/08
- Re: [Bug-gnulib] full-write.c / full-read.c consolidation, Karl Berry, 2003/01/08