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: AS3215 2.6.0.0/16 X-Spam-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI,SPF_HELO_NONE,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by dcvr.yhbt.net (Postfix) with ESMTP id D85BE1F51E for ; Wed, 28 Sep 2022 15:53:38 +0000 (UTC) Authentication-Results: dcvr.yhbt.net; dkim=pass (1024-bit key; unprotected) header.d=pobox.com header.i=@pobox.com header.b="SJdEplLL"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234021AbiI1Pxe (ORCPT ); Wed, 28 Sep 2022 11:53:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40294 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229486AbiI1Px1 (ORCPT ); Wed, 28 Sep 2022 11:53:27 -0400 Received: from pb-smtp1.pobox.com (pb-smtp1.pobox.com [64.147.108.70]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D7344D98E7 for ; Wed, 28 Sep 2022 08:53:26 -0700 (PDT) Received: from pb-smtp1.pobox.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id 23ABC153CCF; Wed, 28 Sep 2022 11:53:26 -0400 (EDT) (envelope-from junio@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type; s=sasl; bh=n6Bp2mcMSaFtRD9WIL1msEaRfkVRpIF+KBvlKF briJo=; b=SJdEplLLf1+zvenxHAdSs9mqOhisDuYDykIg1H5RY8J5SNQofB15vQ D3hm4L+L5n8sBCJDrd5F3SJg67hpvpj95vXcCv5CYswJUD5FGU7q2GzZd4WZ3m3X 2OzqJteG0ybOgYtnU7/Prje1iFqXgnZotI1dlI4XTVKdDe9ltWc7o= Received: from pb-smtp1.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id 1ACD0153CCD; Wed, 28 Sep 2022 11:53:26 -0400 (EDT) (envelope-from junio@pobox.com) Received: from pobox.com (unknown [34.83.5.33]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp1.pobox.com (Postfix) with ESMTPSA id 7EF4C153CCC; Wed, 28 Sep 2022 11:53:25 -0400 (EDT) (envelope-from junio@pobox.com) From: Junio C Hamano To: "Johannes Schindelin via GitGitGadget" Cc: git@vger.kernel.org, Elijah Newren , Taylor Blau , Johannes Schindelin Subject: Re: [PATCH v5 2/2] merge-ort: return early when failing to write a blob References: Date: Wed, 28 Sep 2022 08:53:24 -0700 In-Reply-To: (Johannes Schindelin via GitGitGadget's message of "Wed, 28 Sep 2022 07:29:22 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Pobox-Relay-ID: AFEFD72E-3F45-11ED-B5DA-2AEEC5D8090B-77302942!pb-smtp1.pobox.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org "Johannes Schindelin via GitGitGadget" writes: > This is _not_ just a cosmetic change: Even though one might assume that > the operation would have failed anyway at the point when the new tree > object is written (and the corresponding tree object _will_ be new if it > contains a blob that is new), but that is not so: As pointed out by > Elijah Newren, when Git has previously been allowed to add loose objects > via `sudo` calls, it is very possible that the blob object cannot be > written (because the corresponding `.git/objects/??/` directory may be > owned by `root`) but the tree object can be written (because the > corresponding objects directory is owned by the current user). This > would result in a corrupt repository because it is missing the blob > object, and with this here patch we prevent that. > > Note: This patch adjusts two variable declarations from `unsigned` to > `int` because their purpose is to hold the return value of > `handle_content_merge()`, which is of type `int`. The existing users of > those variables are only interested whether that variable is zero or > non-zero, therefore this type change does not affect the existing code. > > Reviewed-by: Elijah Newren > Signed-off-by: Johannes Schindelin > --- Thanks. The first paragraph in the quoted part above is new and not exactly "reviewed" by Elijah yet, so let's ask ;-) To me the description of the issue looks reasonable to me. Any comments, Elijah? The code is the same as the one in the previous round and Elijah gave his Reviewed-by, and does look good to me, too. > merge-ort.c | 28 +++++++++++++++++++--------- > 1 file changed, 19 insertions(+), 9 deletions(-) Thanks, all. Queued.