Date: Tue, 6 May 2014 13:56:57 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Andrey Chernov <ache@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, "Pedro F. Giffuni" <pfg@freebsd.org>, David Chisnall <theraven@freebsd.org>, src-committers@freebsd.org Subject: Re: svn commit: r265367 - head/lib/libc/regex Message-ID: <20140506133145.G1596@besplex.bde.org> In-Reply-To: <53680532.7050605@freebsd.org> References: <201405051641.s45GfFje086423@svn.freebsd.org> <5367CD77.40909@freebsd.org> <B11B5B25-8E05-4225-93D5-3A607332F19A@FreeBSD.org> <53680532.7050605@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 6 May 2014, Andrey Chernov wrote:
> On 05.05.2014 22:28, David Chisnall wrote:
>> On 5 May 2014, at 18:42, Andrey Chernov <ache@freebsd.org> wrote:
>>
>>> Please don't commit OpenBSD errors. Now you mix calloc() with the
>>> realloc() for the same variable later which makes calloc() zeroing
>>> pointless and waste of CPU.
The existence of calloc() is a bug. OpenBSD might be using it to zero
memory, but this zeroing should be explicit. I didn't really notice
before that the implicit zeroing doesn't even give much shorter code,
since you still need explicit zeroing after realloc() if you have a
policy of zeroing everything.
>> The purpose of calloc() here is not (primarily) to get the zero'd size, it's to get the overflow-checking behaviour for calloc.
It didn't help get the line-length overflow checking in mail.
> It is better to avoid using undocumented intrinsic knowledge of standard
> function particular implementation, this is unportable at least and hard
> to understand too.
It is unportable. The behaviour on overflow is undefined. See another reply.
The overflow checking is painful, but doing it in calloc() is only a
minor simplification. In ps, I needed the following. It is complicated
enough even for system input ({ARG_MAX}) that can be trusted to some
extent.
% if (buf == NULL) {
% if ((arg_max = sysconf(_SC_ARG_MAX)) == -1)
% errx(1, "sysconf _SC_ARG_MAX failed");
% if (arg_max >= LONG_MAX / 4 || arg_max >= (long)(SIZE_MAX / 4))
% errx(1, "sysconf _SC_ARG_MAX preposterously large");
% buf_size = 4 * arg_max + 1;
% if ((buf = malloc(buf_size)) == NULL)
% errx(1, "malloc failed");
% }
Here we can't simply use calloc(4, sysconf(...)) (or is it
calloc(sysconf(...), 4)?) because:
- sysconf() has type long, which differs from size_t, so we can't even
pass sysconf() to calloc() in general
- sysconf() has an out of band error value
- we don't quite have an array, and want to add 1. Adding 1 might overflow.
The above is long partly because it has excessive error handling. This
much error handling is probably justified for user input but not for
sysconf(). Now I don't even like checking if malloc() failed. malloc()
never fails, and if it does then there is nothing much you can do. A
core dump is quite good error handling for it. But error handling like
this allows giving specific error messages. If you use calloc() and
the multiplication overflows, then the behaviour can be anything. If
the behaviour is to return 0 and set errno to indicate the error, then
the best that you can do is hope than errno is set to the undocumented
value EINVAL or EOVERFLOW and then possibly check the multiplication
yourself so as to determine if that was the error, and then print a
specific error message.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140506133145.G1596>