Re: [PATCH] Prepend timestamp in msgbuf


Hi,

On Mon, Oct 17, 2011 at 6:19 PM, Ed Schouten <ed@80386.nl> wrote:
> Hi Arnaud!
>
> * Arnaud Lacombe <lacombar@gmail.com>, 20111017 22:41:
>> + =A0 =A0 =A0 =A0 =A0 =A0 buf[0] =3D '\0';
>> + =A0 =A0 =A0 =A0 =A0 =A0 getnanouptime(&ts);
>> + =A0 =A0 =A0 =A0 =A0 =A0 err =3D snprintf(buf, sizeof buf, "[%zd.%.6ld]=
 ",
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ts.tv_sec, ts.tv_nsec / 1000);
>
> What's the use of buf[0] =3D '\0'? snprintf() will overwrite it anyway,
> right?
leftover from previous debug I guess; fixed.

> Also. please use %jd and cast ts.tv_sec to intmax_t. The size of
> time_t and size_t are independent.
fixed.

> As far as I know, you should be able
> to use a 64-bit time_t on i386 by simply changing the typedef and
> recompiling everything.
>
As long as you do not care about breaking the ABI, yes. But yet, the
kernel and the userland may not need to each have the same
representation of what `time_t' is, as long as they agree on the
interface.

>> + =A0 =A0 =A0 =A0 =A0 =A0 bufp =3D buf;
>> + =A0 =A0 =A0 =A0 =A0 =A0 while (*bufp !=3D '\0') {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 __msgbuf_do_addchar(mbp, seq, =
*bufp);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bufp++;
>> + =A0 =A0 =A0 =A0 =A0 =A0 }
>
> It would be nicer to write this as follows:
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0for (bufp =3D buf; *bufp !=3D '\0'; bufp++=
)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0__msgbuf_do_addchar(mbp, s=
eq, *bufp);
>
fixed.

>> - =A0 =A0 int =A0 =A0 =A0 =A0msg_needsnl; =A0 =A0 =A0 =A0 /* set when ne=
wline needed */
>> + =A0 =A0 uint32_t =A0 msg_flags;
>
> Why change this to uint32_t instead of leaving it the way it is (or
> changing it to unsigned int)? Even though they are likely to be equal in
> size, there is no reason why msg_flags must be 32 bits. :-)
>
made it `unsigned int'; I don't like playing with signed bit-field.

 - Arnaud

> --
> =A0Ed Schouten <ed@80386.nl>
> =A0WWW: http://80386.nl/
>
_______________________________________________
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"