From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.9 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C4B2BC433E0 for ; Fri, 5 Jun 2020 17:40:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9C8052077D for ; Fri, 5 Jun 2020 17:40:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="I8HfI1LK" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727843AbgFERkJ (ORCPT ); Fri, 5 Jun 2020 13:40:09 -0400 Received: from us-smtp-1.mimecast.com ([207.211.31.81]:27826 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726959AbgFERkI (ORCPT ); Fri, 5 Jun 2020 13:40:08 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1591378807; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=/opGoEgIoDnDcLi9yZX38mmT0/AADXDWxmU4z3ixU+s=; b=I8HfI1LKwv557FahYq8nGe7M7yphkM2QmHhRgBgQT62FQ8uVRW82+rYq+vi+29h9kLHJi9 1GbZRwjyxx6P/8ikk1dO1IPCy730BIM/ygHR6SGHRCCTSqg6ptDwcBS16/Mwke2tQ9BFfs pKNTmHiGZ7PYsABP6IIu2V8bgwVg9uE= Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-55-vaL5TAnzP9es3wr10hLWjg-1; Fri, 05 Jun 2020 13:40:01 -0400 X-MC-Unique: vaL5TAnzP9es3wr10hLWjg-1 Received: by mail-qk1-f200.google.com with SMTP id 16so8276197qka.15 for ; Fri, 05 Jun 2020 10:40:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=/opGoEgIoDnDcLi9yZX38mmT0/AADXDWxmU4z3ixU+s=; b=WHxGDzKSRjbFsOGjnRTeOdmCe/pKNKGlnJ96NvvL/Wy0RHvpEof3GNX29sRGwhCYWR mQQY1QaGglXCSb2eku/9s2J8En/wCowuDnopFODghGJPUVAJfN2sIPERgpXv7m0m3v/K 6qKnEhTbau/MQw1vvXoR6KP+vG9fTwMM0aBZ8KjNHx7vYgt1fb6TqSDDEJh+WsoRwJlB ASwHeFJeffNnOhFr/OH7L+MTd+cEiTxiGx7ITlj4dHlbyKnpKCHrfYvNajQVvvs5zDLM yU2E1csYCc+/qIiEesURMazjc93XMOIP2gCXvNpZpjTVdg9tiLQe1kai7aV5f3bz5v+a x8lA== X-Gm-Message-State: AOAM531SZAKNeVNnzdWdHDMzwZsULmQ69GBbhwGTS+lFvpyo75AffjJZ xcrDbtyG+hN8yF8iUonnanWQ7OBBe5mHQrMKkhLHohahQIOaXCaiiSoOlSlFfBxUKuVZKSEoi6M L/22MGMRqqnrU X-Received: by 2002:a0c:b516:: with SMTP id d22mr11035118qve.88.1591378801287; Fri, 05 Jun 2020 10:40:01 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyR45UIs5BGkXfLlHRAVY1bsHugsmO0ud2/c7cAJHdU9j6xjM6xDnoawCzMXujmNEXw/5BqWQ== X-Received: by 2002:a0c:b516:: with SMTP id d22mr11035076qve.88.1591378800779; Fri, 05 Jun 2020 10:40:00 -0700 (PDT) Received: from xz-x1 ([2607:9880:19c0:32::2]) by smtp.gmail.com with ESMTPSA id d13sm328302qkc.121.2020.06.05.10.39.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Jun 2020 10:39:59 -0700 (PDT) Date: Fri, 5 Jun 2020 13:39:58 -0400 From: Peter Xu To: Paolo Bonzini Cc: Andrew Jones , Dan Carpenter , Ben Gardon , Shuah Khan , kvm@vger.kernel.org, linux-kselftest@vger.kernel.org, kernel-janitors@vger.kernel.org Subject: Re: [PATCH] KVM: selftests: delete some dead code Message-ID: <20200605173958.GC71522@xz-x1> References: <20200605110048.GB978434@mwanda> <9f20e25d-599d-c203-e3d4-905b122ef5a3@redhat.com> <20200605115316.z5tavmf5rjobypve@kamzik.brq.redhat.com> <20200605124816.GB55548@xz-x1> <891e89c8-8467-eeb4-1b23-337b88a299dd@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <891e89c8-8467-eeb4-1b23-337b88a299dd@redhat.com> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Fri, Jun 05, 2020 at 03:26:39PM +0200, Paolo Bonzini wrote: > On 05/06/20 14:48, Peter Xu wrote: > >>> The bug is that strtoul is "impossible" to use correctly. > > Could I ask why? > > To see see how annoying the situation is, check out utils/cutils.c in > QEMU; basically, it is very hard to do error handling. From the man page: > > Since strtoul() can legitimately return 0 or ULONG_MAX > (ULLONG_MAX for strtoull()) on both success and failure, the > calling program should set errno to 0 before the call, and then > determine if an error occurred by checking whether errno has > a nonzero value after the call. > > and of course no one wants to write code for that every time they have > to parse a number. > > In addition, if the string is empty it returns 0, and of endptr is NULL > it will accept something like "123abc" and return 123. > > So it is not literally impossible, but it is a poorly-designed interface > which is a major source of bugs. On Rusty's API design levels[1][2], I > would put it at 3 if I'm feeling generous ("Read the documentation and > you'll get it right"), and at -4 to -7 ("The obvious use is wrong") if > it's been a bad day. > > Therefore it's quite common to have a wrapper like > > int my_strtoul(char *p, char **endptr, unsigned long *result); > > The wrapper will: > > - check that the string is not empty > > - always return 0 or -1 because of the by-reference output argument "result" > > - take care of checking that the entire input string was parsed, for > example by rejecting partial parsing of the string if endptr == NULL. > > This version gets a solid 7 ("The obvious use is probably the correct > one"); possibly even 8 ("The compiler will warn if you get it wrong") > because the output argument gives you better protection against overflow. > > Regarding overflow, there is a strtol but not a strtoi, so you need to > have a temporary long and do range checking manually. Again, you will > most likely make mistakes if you use strtol, while my_strtol will merely > make it annoying but it should be obvious that you're getting it wrong. > > Paolo > > [1] https://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html > [2] https://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html Fair enough, and a good reading material. :) Thanks! -- Peter Xu