From owner-svn-src-all@FreeBSD.ORG Tue May 6 03:57:06 2014 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 86BD9413; Tue, 6 May 2014 03:57:06 +0000 (UTC) Received: from mail109.syd.optusnet.com.au (mail109.syd.optusnet.com.au [211.29.132.80]) by mx1.freebsd.org (Postfix) with ESMTP id 45890B39; Tue, 6 May 2014 03:57:05 +0000 (UTC) Received: from c122-106-147-133.carlnfd1.nsw.optusnet.com.au (c122-106-147-133.carlnfd1.nsw.optusnet.com.au [122.106.147.133]) by mail109.syd.optusnet.com.au (Postfix) with ESMTPS id 9F15CD643C4; Tue, 6 May 2014 13:56:58 +1000 (EST) Date: Tue, 6 May 2014 13:56:57 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Andrey Chernov Subject: Re: svn commit: r265367 - head/lib/libc/regex In-Reply-To: <53680532.7050605@freebsd.org> Message-ID: <20140506133145.G1596@besplex.bde.org> References: <201405051641.s45GfFje086423@svn.freebsd.org> <5367CD77.40909@freebsd.org> <53680532.7050605@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=eojmkOZX c=1 sm=1 tr=0 a=7NqvjVvQucbO2RlWB8PEog==:117 a=PO7r1zJSAAAA:8 a=mAlE_CWfnZ8A:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=6I5d2MoRAAAA:8 a=M_1yASzfUti_sNIsEuAA:9 a=CjuIK1q_8ugA:10 a=SV7veod9ZcQA:10 Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, "Pedro F. Giffuni" , David Chisnall , src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 May 2014 03:57:06 -0000 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 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