From: Todd Zullinger <tmz@pobox•com>
To: Jeff King <peff@peff•net>
Cc: "Junio C Hamano" <gitster@pobox•com>,
"Ondrej Pohorelsky" <opohorel@redhat•com>,
"Ondřej Pohořelský via GitGitGadget" <gitgitgadget@gmail•com>,
git@vger•kernel.org
Subject: Re: [PATCH v2] cvsserver: avoid precedence problem between ! and %s
Date: Thu, 22 May 2025 13:56:03 -0400 [thread overview]
Message-ID: <aC9lM12GyntAp2tR@teonanacatl.net> (raw)
In-Reply-To: <20250522170536.GB1613@coredump.intra.peff.net>
Jeff King wrote:
> On Thu, May 22, 2025 at 08:55:56AM -0700, Junio C Hamano wrote:
>
>> Ondrej Pohorelsky <opohorel@redhat•com> writes:
>>
>>>> What made you send a patch for this program? Do you or anybody you
>>>> know use git-cvsserver? Unless I am reading the program
>>>> incorrectly, despite the claim in front of that escapeRefName sub
>>>> that we avoid sending a tag whose name is not something CVS would be
>>>> happy with, we did not sanitize the refs and relied solely on the
>>>> users' repository to use only safe characters in the refs to keep
>>>> CVS clients happy, and the fact that this expression used as if()
>>>> condition is totally broken does not really make any difference,
>>>> since it is in an unused sub. I have to wonder if (1) it is a
>>>> better fix to just remove the unused sub, and/or (2) perhaps nobody
>>>> uses cvsserver to allow cvs clients to talk to a Git repository?
>>
>> Below you mention you found it from test failures. Nice to know
>> that you weren't actually using it ;-)
>>
>> Still, I would welcome second and third set of eyeballs to see if
>> this is a dead code that the "compiler" is complaining about. If
>> so, we can remove that unused code instead of fixing it.
>
> I agree that the code does not appear to be called, and doing this:
>
> diff --git a/git-cvsserver.perl b/git-cvsserver.perl
> index a4e1bad33c..cc891eba67 100755
> --- a/git-cvsserver.perl
> +++ b/git-cvsserver.perl
> @@ -5009,6 +5009,7 @@ sub escapeRefName
> # = "_-xx-" Where "xx" is the hexadecimal representation of the
> # desired ASCII character byte. (for anything else)
>
> + die "foo";
> if(! $refName=~/^[1-9][0-9]*(\.[1-9][0-9]*)*$/)
> {
> $refName=~s/_-/_-u--/g;
>
> still lets t9402 pass. I suspect the issue is that perl complains to
> stderr while parsing the file (polluting the log), not when actually
> running the code.
Just for curiosity, the only commit found with escapeRefName
is when it was added:
$ git log -G '\bescapeRefName\b' -- git-cvsserver.perl
commit 51a7e6dbc9
Author: Matthew Ogilvie <mmogilvi_git@miniinfo•net>
Date: Sat Oct 13 23:42:26 2012 -0600
cvsserver: define a tag name character escape mechanism
CVS tags are officially only allowed to use [-_0-9A-Za-f]. Git
refs commonly uses other characters, especially [./]. Such characters
need to be escaped from CVS in order to be referenced.
This just defines functions to escape/unescape names. The functions
are not used yet.
Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo•net>
Signed-off-by: Junio C Hamano <gitster@pobox•com>
A subsequent commit, 658b57ad52 (cvsserver: add misc commit
lookup, file meta data, and file listing functions,
2012-10-13), made use of unescapeRefName; escapeRefName
seems to have _never_ been used.
--
Todd
next prev parent reply other threads:[~2025-05-22 17:56 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-21 7:45 [PATCH] cvsserver: avoid precedence problem between ! and %s Ondřej Pohořelský via GitGitGadget
2025-05-21 7:53 ` Kristoffer Haugsbakk
2025-05-21 10:23 ` [PATCH v2] " Ondřej Pohořelský via GitGitGadget
2025-05-21 15:54 ` Junio C Hamano
2025-05-21 16:02 ` Junio C Hamano
2025-05-21 17:03 ` Junio C Hamano
2025-05-22 7:19 ` Ondrej Pohorelsky
2025-05-22 15:55 ` Junio C Hamano
2025-05-22 17:05 ` Jeff King
2025-05-22 17:56 ` Todd Zullinger [this message]
2025-05-22 18:51 ` Junio C Hamano
2025-05-23 4:47 ` Matthew Ogilvie
2025-05-23 15:48 ` Junio C Hamano
2025-05-26 13:56 ` Ondrej Pohorelsky
2025-05-27 15:52 ` Junio C Hamano
2025-05-22 11:26 ` [PATCH v3] " Ondřej Pohořelský via GitGitGadget
2025-05-26 13:48 ` [PATCH v4] cvsserver: remove unused escapeRefName function Ondřej Pohořelský via GitGitGadget
2025-05-27 15:24 ` Junio C Hamano
2025-05-21 14:58 ` [PATCH] cvsserver: avoid precedence problem between ! and %s Junio C Hamano
2025-05-21 21:48 ` brian m. carlson
2025-05-22 7:31 ` Ondrej Pohorelsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aC9lM12GyntAp2tR@teonanacatl.net \
--to=tmz@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitgitgadget@gmail$(echo .)com \
--cc=gitster@pobox$(echo .)com \
--cc=opohorel@redhat$(echo .)com \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox