From mboxrd@z Thu Jan 1 00:00:00 1970 From: Junio C Hamano Subject: Re: Buffer overflows Date: Thu, 30 Aug 2007 15:14:04 -0700 Message-ID: <7vtzqg7jrn.fsf@gitster.siamese.dyndns.org> References: <1188502009.29782.874.camel@hurina> <7D84F3C7-129D-4197-AAF1-46298E5D0136@iki.fi> <3f4fd2640708301435s7067137cp5db6334af844158a@mail.gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: "Timo Sirainen" , "Linus Torvalds" , git@vger.kernel.org To: "Reece Dunn" X-From: git-owner@vger.kernel.org Fri Aug 31 00:14:57 2007 Return-path: Envelope-to: gcvg-git@gmane.org Received: from vger.kernel.org ([209.132.176.167]) by lo.gmane.org with esmtp (Exim 4.50) id 1IQsHm-0004ec-FO for gcvg-git@gmane.org; Fri, 31 Aug 2007 00:14:46 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757668AbXH3WOT (ORCPT ); Thu, 30 Aug 2007 18:14:19 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756354AbXH3WOT (ORCPT ); Thu, 30 Aug 2007 18:14:19 -0400 Received: from rune.sasl.smtp.pobox.com ([208.210.124.37]:54885 "EHLO sasl.smtp.pobox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755093AbXH3WOS (ORCPT ); Thu, 30 Aug 2007 18:14:18 -0400 Received: from pobox.com (ip68-225-240-77.oc.oc.cox.net [68.225.240.77]) (using TLSv1 with cipher AES128-SHA (128/128 bits)) (No client certificate requested) by rune.sasl.smtp.pobox.com (Postfix) with ESMTP id E6F0312B629; Thu, 30 Aug 2007 18:14:32 -0400 (EDT) In-Reply-To: <3f4fd2640708301435s7067137cp5db6334af844158a@mail.gmail.com> (Reece Dunn's message of "Thu, 30 Aug 2007 22:35:19 +0100") User-Agent: Gnus/5.110006 (No Gnus v0.6) Emacs/21.4 (gnu/linux) Sender: git-owner@vger.kernel.org Precedence: bulk X-Mailing-List: git@vger.kernel.org Archived-At: "Reece Dunn" writes: > Why is it easier? If you have a fixed-size buffer, why not use > strncpy, which is what a safe string API is essentially doing anyway? I would not claim unchecked strcpy is good -- we obviously would want to fix them. But at the same time use of strncpy, strlcpy and friends solves only half of the problem. Often people say "use strncpy or strlcpy then you would not overstep the buffer", but that does not really solve anything, without additional logic to deal with resulting truncation (barfing with "insanely long string" error message and dying is the least impact). Continuing the work on data that the user did not intend to give you is just as wrong as using corrupt data that overflowed your static buffer. Does Timo's nonstandard API solve that issue? Perhaps it does, perhaps not. Does it make easier to maintain our code? I highly doubt it in the current shape. It is well and widely understood idiom to use strlcpy to a fixed-sized buffer and checking the resulting length to make sure the result would not have overflowed (and if it would have, issue an error and die). I would not have anything against a set of patches to follow such a pattern. But a patch to add a non-standard API that nobody else uses, without any patch to show the changes to a few places that could use the API to demonstrate that the use of API vastly cleans the code up and makes it infinitely harder to make mistakes? The API needs to justify itself to convince the people who needs to learn and adjust to that the benefit far outweighes deviation from better known patterns, and I do not see that happening in Timo's patch.