git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] dir: remove unneeded local variables from match_pathname()

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 10, 2023, 21:51 UTC
Message-ID
<xmqqfsbdgpe1.fsf@gitster.g>
In-Reply-To
<20230210045119.25190-1-masahiroy@kernel.org>
Masahiro Yamada <masahiroy@kernel.org> writes:
> The local variables are unneeded - you can simply advance the 'pathname'
> pointer.

It probably is somewhat subjective if it makes the resulting code easier or harder to read with these extra variables, even though "are unneeded" may technically be correct and the compilers may produce identical binaries with or without the patch.

In the context of the original, this used to be in a loop where elements of an array was matched against a constant pathname variable, and it was necessary to use <name, namelen>, separate variables, to point to the "remainder" of "pathname". It would not have made any sense not to use separate variables in that loop.

When the body of the loop was split into this helper function in b5592632 (exclude: split pathname matching code into a separate function, 2012-10-15), we could have removed these variables and instead clobbered <pathname, pathlen>, but apparently we did not. I suspect that the original author found it easier to reason about the behaviour of the function to keep the incoming parameter anchored at the constant location, and use separate variables to point at the tail part of the string that are to be worked on, which I tend to disagree, but I do not have a strong preference.

Having said all that, I consider this to fall into "once the code is written one way, it is not worth the patch noise to go and change it to a different way." category.

Thanks.
Previous: Masahiro Yamada
Message 2 of 2 in “dir: remove unneeded local variables from match_pathname()”
  1. dir: remove unneeded local variables from match_pathname()Masahiro Yamada, Feb 10, 2023
  2. Junio C HamanoFeb 10, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.