Re: [PATCH 03/15] handle_revision_arg: stop using "dotdot" as a generic pointer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> The handle_revision_arg() function has a "dotdot" variable
> that it uses to find a ".." or "..." in the argument. If we
> don't find one, we look for other marks, like "^!". But we
> just keep re-using the "dotdot" variable, which is
> confusing.
>
> Let's introduce a separate "mark" variable that can be used
> for these other marks. They still reuse the same variable,
> but at least the name is no longer actively misleading.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> It may make sense to pull each of these into its own helper. I didn't
> really look because they're so small, and because the return semantics
> seemed confusing to me. Some of them return, and some of them keep
> parsing. Some of them restore the NUL they overwrite, and some do not.
>
> I didn't dig in to see if there are weird corner cases where they
> misbehave.

I do not quite know what corner cases you meant, but I agree that
many places in the codepath we forget to restore "^" we temporarily
overwrite.  I suspect that none of them is deliberately leaving "^"
unrestored and they are just being careless (or they truly do not
care because they assume nobody will look at arg later).

And I think not restoring cannot be a correct thing to do.  After
all of these parsing, add_rev_cmdline() wants to see arg_ intact.

But let's keep reading; perhaps they are addressed in a later patch,
or they are left as-is, and either is OK.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]