On Wed, Jul 29, 2015 at 05:17:30PM +0100, Rainer Weikusat wrote:
> Isaac Dunham <ibid...@gmail.com> writes:
> > On Wed, Jul 29, 2015 at 01:21:04PM +0100, Rainer Weikusat wrote:
> >> Isaac Dunham <ibid...@gmail.com> writes:
> >> > Or you can do it with mount, sudo, sh, and nlmon 
> >> > (http://git.r-36.net/nlmon).
> >> 
> >> Using poll to wait for a message followed by recvmsg for reading it
> >> offers (in absence of a timeout) no advantages over just doing a
> >> blocking recv. Since this code neither uses control messages nor looks
> >> at the returned flags, it can use recvfrom instead of the (more
> >> complicated) recvmsg. Considering that it checks for message buffer
> >> overflow via len >= sizeof(buf), the MSG_TRUNC flag should probably be
> >> used to make the kernel return the real size in case a datagram was
> >> truncated. Lastly, the message doesn't have to be copied to a 2nd buffer
> >> just to turn the 0-terminated name=value pairs into \n-terminated ones.
> >> 
> >> NB: I admit that I mainly did this because it looked like a nice, simple
> >> programming task.
> >
> > A few comments:
> > - I'm not the maintainer, and I don't see why this got sent here.
> > - There's no reason to factor out print_event_if, and the code it replaced
> >   is clearer and more concise.
> 
> There's a very good reason to do that, namely, it's a separate task
> working with its own set of variables, even in the original code. And

I don't see it as *a* separate task; it's
 - reformat the message
 - decide whether to print the message
 - print the message
which is two or three separate tasks, combined because none of them are
working solely with their own set of variables.

> something which boasts of its light-weightness should keep rescanning

"should *not*" is what you intended to write, I presume?

> the same data over and over again just because that's what the standard
> C library str*-functions will do when used naively or in situations
> they're not well-suited for, eg
> 
> 
> for (i = 0, olen = 0; i < len; i += slen + 1) {
>       slen = strlen(buf+i);
>       if (!slen || !strchr(buf+i, '='))
>               continue;
> 
> At this point, what would really be needed is a function which returns
> a pointer to the next '=' or '\000' but C doesn't provide one.

memchr(buf+i, '\0', sizeof(buf)-i) doesn't work if you want the next char
matching ('=' || '\0'). strchrnul() does what you're asking about, but
isn't portable (GNU only).

A bigger problem is that you need to keep track of the size of the remaining
part of the buffer.
If memcspn() existed, it would be the perfect solution, but it doesn't.

slen = strlen(), then strchr() over the same memory, does involve running
over the same memory twice, but honestly:
you're complaining about going back over 8 bytes of memory?
That's about the longest that a netlink message from the kernel will go
before you hit an '=', unless you're dealing with the start of a packet.
And that's pretty short.

Thanks,
Isaac Dunham
_______________________________________________
Dng mailing list
Dng@lists.dyne.org
https://mailinglists.dyne.org/cgi-bin/mailman/listinfo/dng

Reply via email to