git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Kyle Lippincott <spectral@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] setup: make bareRepository=explicit work in GIT_DIR of a secondary worktree
Date: Fri, 08 Mar 2024 15:32:11 -0800	[thread overview]
Message-ID: <xmqqy1ase1vo.fsf@gitster.g> (raw)
In-Reply-To: <CAO_smVjrKJeKr7QgQWryZRErStFk=Y+1T=dwrR_boXQD_X9_Mg@mail.gmail.com> (Kyle Lippincott's message of "Fri, 8 Mar 2024 15:10:16 -0800")

Kyle Lippincott <spectral@google.com> writes:

>>     $ cat ../seconary/.git
>
> Nit: typo, should be `secondary` (missing the `d`)

Good eyes.  Thanks.

>> +       /*
>> +        * We should be a subdirectory of /.git/worktrees inside
>> +        * the $GIT_DIR of the primary worktree.
>> +        *
>> +        * NEEDSWORK: some folks create secondary worktrees out of a
>> +        * bare repository; they don't count ;-), at least not yet.
>> +        */
>> +       if (!strstr(path, "/.git/worktrees/"))
>
> Do we need to be concerned about path separators being different on
> Windows? Or have they already been normalized here?

I am not certain offhand, but if they need to tolerate different
separators, they can send in patches.

>> +               goto out;
>> +
>> +       /*
>> +        * Does gitdir that points at the ".git" file at the root of
>> +        * the secondary worktree roundtrip here?
>> +        */
>
> What loss of security do we have if we don't have as stringent of a
> check? i.e. if we just did `return !!strstr(path, "/.git/worktrees/)`?

No loss of security.

These "keep result the status we want to return if we want to return
immediately here, and always jump to the out label instead of
returning" patterns is mere a disciplined way to make it easier to
write code that does not leak.  The very first one may not have to
do the "goto out" and instead immediately return, but by writing
this way, I do not need to be always looking out to shoot down
patches that adds new check and/or allocations before this
condition and early "out".

> Or maybe we even combine the existing ends_with(.git) check with this

At the mechanical level, perhaps, but I'd want logically separate
things treated as distinct cases.  One is about being inside
$GIT_DIR of the primary worktrees (where more than majority of users
will encounter) and the new one is about being inside $GIT_DIR of
secondaries.


  reply	other threads:[~2024-03-08 23:32 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-20  0:08 [PATCH] setup: allow cwd=.git w/ bareRepository=explicit Kyle Lippincott via GitGitGadget
2024-01-20 22:26 ` Junio C Hamano
2024-01-22 20:50   ` Kyle Lippincott
2024-03-06 17:27 ` Junio C Hamano
2024-03-08 21:19   ` [PATCH 0/2] Loosening safe.bareRepository=explicit even further Junio C Hamano
2024-03-08 21:19     ` [PATCH 1/2] setup: detect to be in $GIT_DIR with a new helper Junio C Hamano
2024-03-08 21:19     ` [PATCH 2/2] setup: make bareRepository=explicit work in GIT_DIR of a secondary worktree Junio C Hamano
2024-03-08 22:30       ` Junio C Hamano
2024-03-08 23:10       ` Kyle Lippincott
2024-03-08 23:32         ` Junio C Hamano [this message]
2024-03-09  0:12           ` Kyle Lippincott
2024-03-09  1:14             ` Junio C Hamano
2024-03-09  3:20       ` Kyle Meyer
2024-03-09  5:53         ` Junio C Hamano
2024-03-09 23:27     ` [PATCH v2] setup: notice more types of implicit bare repositories Junio C Hamano
2024-03-11 19:23       ` Kyle Lippincott
2024-03-11 21:02         ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqy1ase1vo.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=spectral@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).