From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ea0-x229.google.com (mail-ea0-x229.google.com [IPv6:2a00:1450:4013:c01::229]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mail.saout.de (Postfix) with ESMTPS for ; Fri, 7 Mar 2014 17:11:34 +0100 (CET) Received: by mail-ea0-f169.google.com with SMTP id h14so2447113eaj.0 for ; Fri, 07 Mar 2014 08:11:34 -0800 (PST) Message-ID: <5319EFB4.9020207@gmail.com> Date: Fri, 07 Mar 2014 17:11:32 +0100 From: Milan Broz MIME-Version: 1.0 References: <5318DCEF.5020004@tu-ilmenau.de> <531957DD.7020401@gmail.com> <5319A4DC.6080806@tu-ilmenau.de> In-Reply-To: <5319A4DC.6080806@tu-ilmenau.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [dm-crypt] goto patch for cryptsetup? List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Lars Winterfeld , dm-crypt@saout.de On 03/07/2014 11:52 AM, Lars Winterfeld wrote: > Am 03/07/2014 06:23 AM, schrieb Milan Broz: >> On 03/06/2014 09:39 PM, Lars Winterfeld wrote: >>> Hi, >>> >>> in light of the latest "goto fail"s out there, would you reject a patch >>> replacing all 328 gotos with their semantic equivalents from structured >>> C programming? >> >> cryptsetup code uss goto for only one purpose: to have only one exit >> place from a function (with check for errors, release memory). >> >> Replacing it with anything else means duplicating of lot of code. > > I agree that code duplication is undesirable. I can think of two > alternatives to goto that do not duplicate code. The first is to > introduce a one-time loop like this: No please. Using goto here the style I prefer (you would use exception in other languages but this is intentionally low level C). I really do no understand which problem you are trying to solve here - goto can be used very wrong, but this is not the case. > do { > // foo > if(a) break; // was: goto out; > // bar > if(b) break; // was: goto out; > // "all good" code > return 0; > } while(0); > // was: out: > // "on error" code > return -1; Which is not only ugly but adds one level of indentation. I am basically using kernel code style I like, read https://www.kernel.org/doc/Documentation/CodingStyle (specifically chapter 7 - goto) ... Please do not spend time with replacing goto this way. If you want help - write a tests, there is a lot of things which should have regression tests and still missing there. I would also appreciate script for fuzzy testing, run time checks (to avoid things like gcrypt KDF fail) etc. Running various static/dynamic analyzers and filter out false positives (and send patches for real problems) helps as well. And if anyone understand recent changes in automake, I would appreciate fixes for warnings. (I am thinking also if we could replace it with Cmake:) Thanks, Milan p.s. If the reason for this is recent misapplied patch for OpenSSL, the same problem can happen even without goto. Testsuite should detect such problems... see e.g. http://www.tedunangst.com/flak/post/a-brief-history-of-one-line-fixes