Re: [PATCH 1/2] test-path-utils.c: remove incorrect assumption

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

 



Ray Donnelly <mingw.android@xxxxxxxxx> writes:

>> Some callers of this function in real code (i.e. not the one you are
>> removing the check) do seem to depend on that condition, e.g. the
>> codepath in clone that leads to add_to_alternates_file() wants to
>> make sure it does not add an duplicate, so it may end up not noticing
>> /foo/bar and /foo/bar/ are the same thing, no?  There may be others.
>
> Enforcing that normalize_path_copy() removes any trailing '/' (apart
> from the root directory) breaks other things that assume it doesn't
> mess with trailing '/'s, for example filtering in ls-tree. Any
> suggestions for what to do about this? Would a flag be appropriate as
> to whether to do this part or not? Though I'll admit I don't like the
> idea of adding flags to modify the behavior of something that's meant
> to "normalize" something. Alternatively, I could go through all the
> breakages and try to fix them up?

I agree with you that "normalize" should "normalize".  Making sure
that all the callers expect the same kind of normalization would be
a lot of work but I do think that is the best approach in the long
run.  Thanks for the ls-tree example, by the way, did you find it by
code inspection?  I do not think it is reasonable to expect the test
coverage for this to be 100%, so the "try to fix them up" would have
to involve a lot of manual work both in fixing and reviewing,
unfortunately.

The first step of the "best approach" would be to make a note on
normalize_path_copy() by adding a NEEDSWORK: comment to describe the
situation.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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]