* runit: buffer_feed()
@ 2009-01-23 13:06 Tomas Carnecky
2009-01-23 14:30 ` Charlie Brady
0 siblings, 1 reply; 4+ messages in thread
From: Tomas Carnecky @ 2009-01-23 13:06 UTC (permalink / raw)
To: supervision
I'm looking at the buffer_feed() function in runit and I can't make
sense of it. I'm in the process of replacing all byte_* functions with
standard memcpy/memcmp etc, but I just don't see what this byte_copyr()
is supposed to do:
int buffer_feed(buffer *s)
{
int r;
if (s->p) return s->p;
r = oneread(s->op,s->fd,s->x,s->n);
if (r <= 0) return r;
s->p = r;
s->n -= r;
if (s->n > 0) byte_copyr(s->x + s->n,r,s->x);
return r;
}
byte_copyr() copies backwards, so you're reading/accessing bytes
_before_ s->x. Shouldn't that cause a SIGSEGV?
Also, what is ->p and ->n? ->n seems to be the offset into the ->s char
array, ->p the number of bytes currently in the buffer. It looks that
way if I look at buffer_peek(), but ->n is initialized to 'len', which
means that buffer_peek() just after the buffer is initialized returns a
pointer beyond the end of the buffer.. ? Also, ->n is decreased after
data is read into the buffer (using oneread). That doesn't make sense.
It's all very confusing.
Can sombody please explain how the code is supposed to work?
thanks
tom
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: runit: buffer_feed()
2009-01-23 13:06 runit: buffer_feed() Tomas Carnecky
@ 2009-01-23 14:30 ` Charlie Brady
2009-01-23 14:57 ` Tomas Carnecky
0 siblings, 1 reply; 4+ messages in thread
From: Charlie Brady @ 2009-01-23 14:30 UTC (permalink / raw)
To: Tomas Carnecky; +Cc: supervision
On Fri, 23 Jan 2009, Tomas Carnecky wrote:
> I'm looking at the buffer_feed() function in runit and I can't make sense of
> it. I'm in the process of replacing all byte_* functions with standard
> memcpy/memcmp etc,
Why?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: runit: buffer_feed()
2009-01-23 14:30 ` Charlie Brady
@ 2009-01-23 14:57 ` Tomas Carnecky
2009-01-23 17:06 ` Laurent Bercot
0 siblings, 1 reply; 4+ messages in thread
From: Tomas Carnecky @ 2009-01-23 14:57 UTC (permalink / raw)
To: Charlie Brady; +Cc: supervision
On 01/23/2009 03:30 PM, Charlie Brady wrote:
>
> On Fri, 23 Jan 2009, Tomas Carnecky wrote:
>
>> I'm looking at the buffer_feed() function in runit and I can't make
>> sense of it. I'm in the process of replacing all byte_* functions with
>> standard memcpy/memcmp etc,
>
> Why?
byte_copy is a poor reimplementation of memcpy, and memcpy is part of
any libc runtime (according to the man page, conforming to: SVr4,
4.3BSD, C89, C99, POSIX.1-2001.). Please point my to a single libc that
implements malloc, open, write, fork but not memcpy.
Also, it makes the executable smaller and faster, the project as a whole
faster to compile, the code easier to read etc.
But nvm, I found out what I was missing. The code still doesn't make
sense to me, but at least the resulting executables works correctly.
tom
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: runit: buffer_feed()
2009-01-23 14:57 ` Tomas Carnecky
@ 2009-01-23 17:06 ` Laurent Bercot
0 siblings, 0 replies; 4+ messages in thread
From: Laurent Bercot @ 2009-01-23 17:06 UTC (permalink / raw)
To: supervision
> byte_copy is a poor reimplementation of memcpy
Wrong.
byte_copy works when the source and destination overlap and dst < src.
byte_copyr works when the source and destination overlap and dst > src.
memcpy is NOT guaranteed to work when the source and destination overlap.
If you want an optimized function that works in every case, try memmove.
> Also, it makes the executable smaller and faster, the project as a whole
> faster to compile, the code easier to read etc.
Your mileage may vary. I'm used to the byte_copy() interface. :)
I'm not pretending your remarks are unfounded. The functions in byte.h
and buffer.h are very old DJB code, when he was rewriting the world and
its mother in order to be portable on alien computers pretending to be
half POSIX-compliant.
I'm not aware of any modern architecture/libc combination where memmove()
doesn't work, so I *think* byte_copy and byte_copyr's naive implementations
can be safely replaced with memmove() (and that's what skalibs does by
default). I could be wrong, though, and be non-portable on a few archs.
As for buffer.h, I disliked DJB's implementation and rewrote it entirely.
The API, however, is excellent.
(If I'm not mistaken, buffer_feed() fills the buffer if it's empty (and
doesn't if it's not), and returns the number of bytes in the buffer.)
What I'm saying is there's risk involved in patching software without
giving it a good deal of thought - especially software including DJB code,
which usually features a number of non-apparent, but important, design
decisions. If you go and wildly replace byte_copy with memcpy, you will
introduce bugs into the code. ;)
--
Laurent
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-01-23 17:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-01-23 13:06 runit: buffer_feed() Tomas Carnecky
2009-01-23 14:30 ` Charlie Brady
2009-01-23 14:57 ` Tomas Carnecky
2009-01-23 17:06 ` Laurent Bercot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).