From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-3.7 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.6 Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org [IPv6:2604:1380:45e3:2400::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 34D471F44D for ; Tue, 23 Apr 2024 21:19:49 +0000 (UTC) Authentication-Results: dcvr.yhbt.net; dkim=pass (1024-bit key; unprotected) header.d=pobox.com header.i=@pobox.com header.a=rsa-sha256 header.s=sasl header.b=Uwr4x3gq; dkim-atps=neutral Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id AC38C28EB0F for ; Tue, 23 Apr 2024 21:19:47 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 5731614387B; Tue, 23 Apr 2024 21:19:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b="Uwr4x3gq" Received: from pb-smtp2.pobox.com (pb-smtp2.pobox.com [64.147.108.71]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 83270143866 for ; Tue, 23 Apr 2024 21:19:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=64.147.108.71 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713907173; cv=none; b=sFPyYYlvd3P5V/NfNztqkeS+EHSkLNLH6EH2PNEtAOMwbNsDFE7rP1rvTtTKthyX5lz9YefqVjzIEbjSY6chpC2oF1AcBJac3jTflrHt+cW6F1ftUKnoh/TqLgQsBXO3eTOEsw8MWT7RwF+pFUstK+10gnDZMAAi2HDvidIX/Wg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1713907173; c=relaxed/simple; bh=/M0kXJ28tffQinFlMyKjl5KzAKvKmQ/5sNM7cw2bsUI=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=iJRj1OIvoCIK8t5/DiG9BGeBGJwU7jmmZwFeq9MUOP2T9RDSpq8AeLM6N8DvR5q66w88eMao9KZbn6LuBJjEtl9BT4tEZxoEdql6Ps/QCmBXbq4qqV0KYSy+U2nOV9Ht9+U8TvlzuXAhwU3ldYw/Nrn5Oz013ZpGFqPbD7RNS5c= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com; spf=pass smtp.mailfrom=pobox.com; dkim=pass (1024-bit key) header.d=pobox.com header.i=@pobox.com header.b=Uwr4x3gq; arc=none smtp.client-ip=64.147.108.71 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=pobox.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pobox.com Received: from pb-smtp2.pobox.com (unknown [127.0.0.1]) by pb-smtp2.pobox.com (Postfix) with ESMTP id 48F571F546F; Tue, 23 Apr 2024 17:19:24 -0400 (EDT) (envelope-from junio@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to:cc :subject:in-reply-to:references:date:message-id:mime-version :content-type; s=sasl; bh=/M0kXJ28tffQinFlMyKjl5KzAKvKmQ/5sNM7cw 2bsUI=; b=Uwr4x3gqfVGz10ZLC3Wevo8V0GCT53A69Ckka3DBHIUjxXCldbJq9g Mzsanl8d3CQKoVyMBmONqREw6NUSfZMHmaOg4Nq4ux/a3uUupUhb0ndAi3TD29BI man/bRqxfBv/xNXk2pUv1MaDAa0xCb0IR4Hq4WkckAvJkjRfs9g3I= Received: from pb-smtp2.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp2.pobox.com (Postfix) with ESMTP id 3F5761F546E; Tue, 23 Apr 2024 17:19:24 -0400 (EDT) (envelope-from junio@pobox.com) Received: from pobox.com (unknown [34.125.120.109]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp2.pobox.com (Postfix) with ESMTPSA id A05541F546D; Tue, 23 Apr 2024 17:19:23 -0400 (EDT) (envelope-from junio@pobox.com) From: Junio C Hamano To: "Linus Arver via GitGitGadget" Cc: git@vger.kernel.org, Christian Couder , Emily Shaffer , Josh Steadmon , "Randall S. Becker" , Christian Couder , Kristoffer Haugsbakk , Linus Arver Subject: Re: [PATCH v2 4/8] sequencer: use the trailer iterator In-Reply-To: <84897cf5c83eb67c023603016b49fb7b56870aa3.1713504153.git.gitgitgadget@gmail.com> (Linus Arver via GitGitGadget's message of "Fri, 19 Apr 2024 05:22:29 +0000") References: <84897cf5c83eb67c023603016b49fb7b56870aa3.1713504153.git.gitgitgadget@gmail.com> Date: Tue, 23 Apr 2024 14:19:22 -0700 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain X-Pobox-Relay-ID: 28320F92-01B7-11EF-B223-25B3960A682E-77302942!pb-smtp2.pobox.com "Linus Arver via GitGitGadget" writes: > From: Linus Arver > > Instead of calling "trailer_info_get()", which is a low-level function > in the trailers implementation (trailer.c), call > trailer_iterator_advance(), which was specifically designed for public > consumption in f0939a0eb1 (trailer: add interface for iterating over > commit trailers, 2020-09-27). > > Avoiding "trailer_info_get()" means we don't have to worry about options > like "no_divider" (relevant for parsing trailers). We also don't have to > check for things like "info.trailer_start == info.trailer_end" to see > whether there were any trailers (instead we can just check to see > whether the iterator advanced at all). > > Note how we have to use "iter.raw" in order to get the same behavior as > before when we iterated over the unparsed string array (char **trailers) > in trailer_info. > Signed-off-by: Linus Arver > --- > sequencer.c | 27 ++++++++++++--------------- > 1 file changed, 12 insertions(+), 15 deletions(-) OK. The code reduction primarily comes from the fact that the original was manually iterating over the trailer lines that you can get from the iterator API. > + while (trailer_iterator_advance(&iter)) { > + i++; > + if (sob && !strncmp(iter.raw, sob->buf, sob->len)) > + found_sob = i; > + } > + trailer_iterator_release(&iter); > > + if (!i) > + return 0; > > + found_sob_last = (int)i == found_sob; This is slightly harder to reason about, as we cannot directly say "the collection being iterated over has .nr members, and what we found was at the end" like the original could do in its loop. > - for (i = 0; i < info.trailer_nr; i++) > - if (sob && !strncmp(info.trailers[i], sob->buf, sob->len)) { > - found_sob = 1; > - if (i == info.trailer_nr - 1) > - found_sob_last = 1; > - } As 'i' is incremented before we set found_sob to it in the new loop, when it is assigned in the loop, the value of found_sob will never be zero. It used to be that found_sob takes only 0 or 1, but because we only care about found_sob and found_sob_last being zero/non-zero in the remainder of the code, this does not affect the correctness of the code. Looking good.