Date: Mon, 17 Oct 2011 20:16:07 -0400 From: Arnaud Lacombe <lacombar@gmail.com> To: Ed Schouten <ed@80386.nl> Cc: FreeBSD Current <freebsd-current@freebsd.org> Subject: Re: [PATCH] Prepend timestamp in msgbuf Message-ID: <CACqU3MUyAa3D_WZM0ABgi7Fo-2hDxvn_xhdkb3NKn--sqg9pBw@mail.gmail.com> In-Reply-To: <20111017221933.GZ91943@hoeg.nl> References: <1318884099-6005-1-git-send-email-lacombar@gmail.com> <20111017221933.GZ91943@hoeg.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
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/
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CACqU3MUyAa3D_WZM0ABgi7Fo-2hDxvn_xhdkb3NKn--sqg9pBw>