Re: [PATCH] Prepend timestamp in msgbuf

Hi Arnaud!

* Arnaud Lacombe <lacombar_at_gmail.com>, 20111017 22:41:
> +		buf[0] = '\0';
> +		getnanouptime(&ts);
> +		err = snprintf(buf, sizeof buf, "[%zd.%.6ld] ",
> +		    ts.tv_sec, ts.tv_nsec / 1000);

What's the use of buf[0] = '\0'? snprintf() will overwrite it anyway,
right? Also. please use %jd and cast ts.tv_sec to intmax_t. The size of
time_t and size_t are independent. 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.

> +		bufp = buf;
> +		while (*bufp != '\0') {
> +			__msgbuf_do_addchar(mbp, seq, *bufp);
> +			bufp++;
> +		}

It would be nicer to write this as follows:

		for (bufp = buf; *bufp != '\0'; bufp++)
			__msgbuf_do_addchar(mbp, seq, *bufp);

> -	int	   msg_needsnl;		/* set when newline needed */
> +	uint32_t   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. :-)

-- 
 Ed Schouten <ed_at_80386.nl>
 WWW: http://80386.nl/