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=-5.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 96514C433ED for ; Thu, 15 Apr 2021 09:34:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 636EE61153 for ; Thu, 15 Apr 2021 09:34:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232030AbhDOJe6 (ORCPT ); Thu, 15 Apr 2021 05:34:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58058 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231531AbhDOJe5 (ORCPT ); Thu, 15 Apr 2021 05:34:57 -0400 Received: from mail-il1-x134.google.com (mail-il1-x134.google.com [IPv6:2607:f8b0:4864:20::134]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 18DF6C061756 for ; Thu, 15 Apr 2021 02:34:34 -0700 (PDT) Received: by mail-il1-x134.google.com with SMTP id 7so17964466ilz.0 for ; Thu, 15 Apr 2021 02:34:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=2oxhfmgq4/lBHZsTv5waxIB12jAdndxIiCGYaYJ0QbU=; b=UbdMnzkMjlp4dVJzE5P6mIlHvL1lwGcw1qltoT9qncf5Oc0r2fUgB9SKUW0wR9lDVC tKEZRlsxRg6hWGuydX2yQhRoWDikqmWFp5mOfPESNP0UQuEbY0Z7R5UaUJ+XT9zQ6g7R /KAeiqhfw8gm2oDkm/y60F44SPj0DkvpIy+BE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=2oxhfmgq4/lBHZsTv5waxIB12jAdndxIiCGYaYJ0QbU=; b=o7isWD3Ulu5Lu8OamTOIwOxa2atODwSRicVZ7KvLLFF1liYYv5O2yKYZJRGgbSptVU TI0OIJ9a2WCDJdgh6y4E+mEdAhvSxjclsnNgOxRGr/16rYV1T8E1Xi5ybplzlnorlM++ hJglWrIHls+WKunFgNBuCOw+d5bN3qbCwDWZ6i/gZNoqlEopBN4X+VbByyJ8v0Fv18zN HLeDQLJyjx4+MMDek+ojKmjwHaNIN2YnaTvIbWuEXZDYsQewLfPMQ4dIaCUxsBIDjaTl GYHcfXflLG9qg81kJlaUbH9Nx/n8H91jdkmRrrZNr446pUq5vscLrcSYd6fpGRXrhJvt Aq2w== X-Gm-Message-State: AOAM530LYBc9/H0+q94f34WK2TeUdXMllCFbRBJYVUKLIbBhUnyMBuwK XhsO5MFOp87zS6FlcBlirDSrckjmSiyg0i8XEma3gw== X-Google-Smtp-Source: ABdhPJzeShXmcoEfu+1+Ri6SonH6CoZJyCAHTZk+mfvL5aCtUdErvhGRLF2ppMLjZe5OeRF+Nqbi3GEmTsfRatK8tD4= X-Received: by 2002:a92:ce90:: with SMTP id r16mr2135098ilo.220.1618479273627; Thu, 15 Apr 2021 02:34:33 -0700 (PDT) MIME-Version: 1.0 References: <20210412153754.235500-1-revest@chromium.org> <20210412153754.235500-4-revest@chromium.org> In-Reply-To: From: Florent Revest Date: Thu, 15 Apr 2021 11:34:22 +0200 Message-ID: Subject: Re: [PATCH bpf-next v3 3/6] bpf: Add a bpf_snprintf helper To: Andrii Nakryiko Cc: bpf , Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Yonghong Song , KP Singh , Brendan Jackman , open list Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On Thu, Apr 15, 2021 at 12:57 AM Andrii Nakryiko wrote: > > On Wed, Apr 14, 2021 at 2:46 AM Florent Revest wrote: > > > > On Wed, Apr 14, 2021 at 1:16 AM Andrii Nakryiko > > wrote: > > > On Mon, Apr 12, 2021 at 8:38 AM Florent Revest wrote: > > > > + > > > > + return err + 1; > > > > > > snprintf() already returns string length *including* terminating zero, > > > so this is wrong > > > > lib/vsprintf.c says: > > * The return value is the number of characters which would be > > * generated for the given input, excluding the trailing null, > > * as per ISO C99. > > > > Also if I look at the "no arg" test case in the selftest patch. > > "simple case" is asserted to return 12 which seems correct to me > > (includes the terminating zero only once). Am I missing something ? > > > > no, you are right, but that means that bpf_trace_printk is broken, it > doesn't do + 1 (which threw me off here), shall we fix that? Answered in the 1/6 thread > > However that makes me wonder whether it would be more appropriate to > > return the value excluding the trailing null. On one hand it makes > > sense to be coherent with other BPF helpers that include the trailing > > zero (as discussed in patch v1), on the other hand the helper is > > clearly named after the standard "snprintf" function and it's likely > > that users will assume it works the same as the std snprintf. > > > Having zero included simplifies BPF code tremendously for cases like > bpf_probe_read_str(). So no, let's stick with including zero > terminator in return size. Cool :)