From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 18 Feb 2020 00:35:45 +0300 From: "Dmitry V. Levin" To: ALT Devel discussion list Message-ID: <20200217213545.GA18365@altlinux.org> References: <2007feb6-b08c-b463-a0fe-d51a264bee45@altlinux.org> MIME-Version: 1.0 Content-Type: text/plain; charset=koi8-r Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: Re: [devel] re APT patch with impossible error on "Can't allocate an item of size zero" X-BeenThere: devel@lists.altlinux.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: ALT Linux Team development discussions List-Id: ALT Linux Team development discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Feb 2020 21:35:45 -0000 Archived-At: List-Archive: List-Post: On Mon, Feb 17, 2020 at 11:41:36AM +0300, Ivan Zakharyaschev wrote: > On Thu, 13 Feb 2020, Ivan Zakharyaschev wrote: > > > > > есть такой hunk касательно DynamicMMap::RawAllocate(): > > > > > > > > diff --git a/apt/apt-pkg/contrib/mmap.cc b/apt/apt-pkg/contrib/mmap.cc > > > > index d7a5c3a68..36dc11524 100644 > > > > --- a/apt/apt-pkg/contrib/mmap.cc > > > > +++ b/apt/apt-pkg/contrib/mmap.cc > > > > @@ -226,6 +245,12 @@ unsigned long DynamicMMap::RawAllocate(unsigned long > > > > Size,unsigned long Aln) > > > > size in the file. */ > > > > unsigned long DynamicMMap::Allocate(unsigned long ItemSize) > > > > { > > > > + if (ItemSize == 0) > > > > + { > > > > + _error->Error("Can't allocate an item of size zero"); > > > > + return 0; > > > > + } > > > > + > > > > // Look for a matching pool entry > > > > Pool *I; > > > > Pool *Empty = 0; > > > > > Во-вторых, я подумал, что это такой класс ошибок, который не возникает > > > > в run-time из-за плохих данных или отказа ОС (например, выделить > > > > память или прочитать файл) -- т.е. не что-то заранее непредсказуемое > > > > для программиста и поэтому требующее специальной обработки в run-time. > > > > > > > > Если програмист написал всё так, как он хотел, то он не передаст > > > > неправильный аргумент (0) в функцию. (Да и понятно, что в реальном > > > > коде APT это не ожидается: там обычно этот аргумент заполняется > > > > константой sizeof(...).) > > > > > > > > > > Ты упускаешь большой момент. mmap.h - публичный заголовок библиотеки libapt. > > > Помимо самого apt, им пользуется ещё и немало клиентов. Даже если в самом apt > > > не найдётся способа неправильно вызвать данную функцию, это ничего не значит. > > > Не проверять входные данные - плохая идея. > > > > Всё равно это ответственность программиста: правильно вызвать. > > > > Я выше написал, в каких случаях я счиатю нужным проверять данные. > > > > > > Я бы предложил такие требования (имеющие характер спецификации того, > > > > что программист задумал реализовать) оформлять просто с помощью > > > > assert. > > > > > > > > > > assert в не-debug сборке отсутствует. Потому чуть более чем полностью > > > бесполезен не во время разработки приложения. > > > > Я так и задумывал. > > > > > > > Это не средство проверки входных > > > данных. Поэтому я против. > > > > > > > Так будет легче понимать код: сразу ясно, что выполнение этого условия > > > > (важного для корректности кода в теле функции) просто гарантируется > > > > тем, как весь наш код написан и вообще-то может быть обосновано ещё до > > > > run-time (просто глядя на исходный код, на том же этапе работы, что и > > > > запускаем компилятор). > > Появилась идея, что есть способ здесь не просто ответственность возложить > на программиста на словах (вызывать Allocate() с ненулевым аргументом), но > и формально предложить такой способ программирования, где не будет > возможности делать не так, как задумано. > > Да и попроще вызовы получаются. > > Ветка alloc-templates в > http://git.altlinux.org/people/imz/packages/apt.git Я всячески приветствую такой подход, и я всем советую static_assert. Но всё же static_assert(sizeof(T) > 0, "sizeof(T) == 0") выглядит чрезмерно пессимистичным. Надо всё-таки очень сильно постараться, чтобы инстанцировать pkgCacheGenerator::AllocateInMap() классом нулевого размера. -- ldv