From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS53758 23.128.96.0/24 X-Spam-Status: No, score=-11.6 required=3.0 tests=AWL,BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,SPF_PASS, USER_IN_DEF_DKIM_WL shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id AC1EB1F852 for ; Wed, 12 Jan 2022 11:40:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240288AbiALLkC (ORCPT ); Wed, 12 Jan 2022 06:40:02 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36622 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239705AbiALLkB (ORCPT ); Wed, 12 Jan 2022 06:40:01 -0500 Received: from mail-ua1-x92f.google.com (mail-ua1-x92f.google.com [IPv6:2607:f8b0:4864:20::92f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 40466C06173F for ; Wed, 12 Jan 2022 03:40:01 -0800 (PST) Received: by mail-ua1-x92f.google.com with SMTP id y4so4182501uad.1 for ; Wed, 12 Jan 2022 03:40:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=KSqWPBadiYwofZgHf47PsYzzDhBYuaV9LDe8geDRQOc=; b=aBUtE8B9RfMGfzxXjL5KmZDTAjn3GoGmvW1VNWB+/HJtnEeTyt+CZMLW1ALD9ncBmo EboKeCe6eXotknFIhBU1RvTF0UxayI3lGpwOOJqOzJZtZT7klS0AlAGvwvfwHbicC8dg vulqjbSlRZI5zHgars408p9sSKA1M0noRhJML+eUZ6itLI9trQrGoN8fDnexYrmxMSeX 6ETqbwP72Fpu92ea5++T7CCGzHpuO+JsiG8i8wpHlPm0d0sh1Etd8t8L8JTaIXHz9vpn PetbIsWd0LH/s+v+yIEh1u3ZoU1jes8BYXED1dTEJWYMkddZBvm4J66BNlVeeCAdE98S tNAA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=KSqWPBadiYwofZgHf47PsYzzDhBYuaV9LDe8geDRQOc=; b=xh1B4CIzs4N13U163GRdyoYA5Zp3MQngqZG+WY8Cw0LKJMPZZPxQOmpFLCg17FTJd2 8KDR1fcCqUcn319LIOnfoQvRjuzD4w3q9zB9iKrS4t0Qrv9l/m/w70Omy73BwYZxc5bv PaM9mGZnM4riJO3dccXZlqHOpx7Y5W5DepTGla8xJkp9vwf2nzc3ntdEfq/HNgUQWW6o eLxBbGdBMYtDqn1zTx6QYJPzVibhzOAX0XaKPoJhza6LIFTU/iWpM22groSyc00jRWfm doysD9Qo+dykwpa3Y2MU9YcyaJfgg4GOoH80SO5ch7QTH24YS4gDvkZKfEmasJS0I+z8 pfzw== X-Gm-Message-State: AOAM532x4IVz3RDNZd6BBoPx2bMlbq+IUiY/I5sP0TqFSNXgNP9uyMw2 4CSwqKXUMZ02R4whUHYE7HzfEoMzT5CJHLO3Uhi3MA== X-Google-Smtp-Source: ABdhPJxNs+Jpn3PCadr/EW0zIwOml/yq1/MDrTo10gWiZA6lufIUqZM+uvTxFlduN/nllmYu7L2cnvz4BdV0irPKCwY= X-Received: by 2002:a67:f497:: with SMTP id o23mr4582154vsn.70.1641987600338; Wed, 12 Jan 2022 03:40:00 -0800 (PST) MIME-Version: 1.0 References: In-Reply-To: From: Han-Wen Nienhuys Date: Wed, 12 Jan 2022 12:39:49 +0100 Message-ID: Subject: Re: [PATCH v5 16/16] reftable: be more paranoid about 0-length memcpy calls To: Junio C Hamano Cc: Han-Wen Nienhuys via GitGitGadget , git@vger.kernel.org, Jeff King , =?UTF-8?B?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= , Han-Wen Nienhuys Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Fri, Dec 24, 2021 at 5:16 AM Junio C Hamano wrote: > > But your safe_memcpy() should not be > > safe_memcpy(dst, src, n) > { > if (n) > memcpy(dst, src, n); > return dst; > } > > Using memcpy() with size=3D=3D0 is not a crime wrt the language. > Passing an invalid pointer while doing so is. It's not a crime, but what is the benefit of calling memcpy with n =3D=3D 0= ? > safe_memcpy(dst, src, n) > { > if (dst) > memcpy(dst, src, n); > else if (n) > BUG(...); I think this is suboptimal. Sure, a segfault is uglier than "out of memory" error, but both effectively crash the program, so the difference isn't that big. The nice way is if the reftable library grows an error-code REFTABLE_OOM, which is propagated once a malloc or realloc returns NULL. We could test this exhaustively in the unittest by swapping in a malloc that starts failing after N allocations, and then running a transaction in a loop, increasing N. I'll have to look more closely if this is possible throughout, so for this series, I'll just take a closer look at the current call-sites to see if NULL can really occur or not. --=20 Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Gesch=C3=A4ftsf=C3=BChrer: Paul Manicle, Halimah DeLaine Prado