Send inn-workers mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.isc.org/mailman/listinfo/inn-workers
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of inn-workers digest..."
Today's Topics:
1. Re: [PATCH] fix snprintf return value misuse (and some
related off-by-1/etc) (Yuriy M. Kaminskiy)
----------------------------------------------------------------------
Message: 1
Date: Mon, 5 Sep 2016 16:35:45 +0300
From: "Yuriy M. Kaminskiy" <[email protected]>
To: [email protected]
Subject: Re: [PATCH] fix snprintf return value misuse (and some
related off-by-1/etc)
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252; format=flowed
On 01.09.2016 22:31, Julien ?LIE wrote:
[skip]
> --- lib/timer.c (revision 9984)
> +++ lib/timer.c (working copy)
> @@ -369,14 +387,36 @@
>
> - off += snprintf(buf + off, len - off, "time %lu ", TMRgettime(true));
> + rc = snprintf(buf + off, len - off, "time %lu ", TMRgettime(true));
> + if (rc < 0) {
> + /* do nothing */
> + } else if ((size_t)rc >= len) {
> + off = len;
> + } else {
> + off += rc;
> + }
>
> Hmm, are you sure the check is "if ((size_t)rc >= len)"
> and not "if ((size_t)rc >= len - off)"?
Ugrh. Sorry. You are right, and your variant is correct.
> I think "len - off" is really needed because otherwise the returned
> value could be larger than the supplied buffer size of "len - off".
>
>
>
> --- lib/timer.c (revision 9984)
> +++ lib/timer.c (working copy)
> @@ -359,6 +376,7 @@
>
> for (i = 0; i < timer_count; i++)
> - if (timers[i] != NULL)
> - off += TMRsumone(labels, timers[i], buf + off, len - off);
> + if (timers[i] != NULL) {
> + rc = TMRsumone(labels, timers[i], buf + off, len - off);
> + if (rc < 0) {
> + /* do nothing */
> + } else if ((size_t)rc >= len) {
(FWIW, same bug here, it should've been
+ } else if ((size_t)rc >= len - off) {
)
> + off = len;
> + } else {
> + off += rc;
> + }
> + }
>
> Is that change really necessary, as TMRsumone() can no longer return
> a negative value or a value greater than the supplied buffer size?
Well, yes, this change is redundant with changed TMRsumone().
(I tried to be consistent with snprintf()-calling code, but now that I
looked at it again, it is not consistent with other TMRsumone call-sites
anyway; so, this chunk should be dropped).
Thanks for review.
> I would just not change anything there.
>
> Thanks again for your work,
------------------------------
Subject: Digest Footer
_______________________________________________
inn-workers mailing list
[email protected]
https://lists.isc.org/mailman/listinfo/inn-workers
------------------------------
End of inn-workers Digest, Vol 87, Issue 3
******************************************
