Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 18 Oct 2011 00:19:33 +0200
From:      Ed Schouten <ed@80386.nl>
To:        Arnaud Lacombe <lacombar@gmail.com>
Cc:        FreeBSD Current <freebsd-current@freebsd.org>
Subject:   Re: [PATCH] Prepend timestamp in msgbuf
Message-ID:  <20111017221933.GZ91943@hoeg.nl>
In-Reply-To: <1318884099-6005-1-git-send-email-lacombar@gmail.com>
References:  <1318884099-6005-1-git-send-email-lacombar@gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help

--36Vc0Wavwf33RWLQ
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi Arnaud!

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

What's the use of buf[0] =3D '\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 =3D buf;
> +		while (*bufp !=3D '\0') {
> +			__msgbuf_do_addchar(mbp, seq, *bufp);
> +			bufp++;
> +		}

It would be nicer to write this as follows:

		for (bufp =3D buf; *bufp !=3D '\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. :-)

--=20
 Ed Schouten <ed@80386.nl>
 WWW: http://80386.nl/

--36Vc0Wavwf33RWLQ
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (FreeBSD)

iQIcBAEBAgAGBQJOnKn1AAoJEG5e2P40kaK7gZgP/0AuodFCwpIo8lpU/4a3sH85
vK7iEvls8dq6Aehp//63zynHHMvAVg90m1XatsjIFc2xKRj3gsi17BaPFOb9WoG7
4O0kmIMO8Lw8NU4Kgz83oF7c+jGqZEocELjcnd9fYSOKhSQWzuzas9bDvLpYrxEo
Rv2Vh5My1HWWtJu+mmbepxkzMzXGO0sVdOlI5eHHuOcs66Om9M0ePP/gVKKS6m5r
rjdFQZh1MDvXuGHr2HeXavAHI7CZ46VZ6uGGOXqKnpgvRRYqPBX/26wdZAB/5kn0
1bGqfHyAW9lRGO8sUL1peDpZ+08caU0+9VeEoT2Ywi56bUlKVsHD6JG+2X8oCBhR
LCAAVIgD72r7dHSntkvDJHFO6J5BJafhCKJuzhmnavSLQfDNqMQLX9GfyidHDjIz
/19BZHQ3q+rQS2TIeMpjRHa8rwklTIpMPMqjZ201e/lnxR/Pj6gqCJuIT00uG8Ck
bb4xcLFBlyTW/f6/vlCTripFjx4IkhHJtX+ru3Gml0GtoLG0NmbJmUtHBlfamfZe
uWaQfses6brEhYOfE4q86qXM6tvIPbFoc9cBuOojBUAEgl8WdSUYWvn0ALIEcrIO
Vsio31yUXdVACa3mKLo32DDmSXI38N9iYdFyr0+G0gKZSVjmqUGaGiCYu5Q2Qaf5
73O2B4fWV1sCZH49uX3x
=5FeF
-----END PGP SIGNATURE-----

--36Vc0Wavwf33RWLQ--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111017221933.GZ91943>