Re: [PATCH 2/6] gitweb: Simplify git_diff_print

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

 



Jakub Narebski <jnareb@xxxxxxxxx> writes:

> Copy to temporaty file more directly, not using temporary variable @file.
> Use list form of open for diff invocation (we cannot use git-diff because
> first it doesn't support -L/--label option, and we cannot generate diff
> between /dev/null and blob given by it's sha1 identifier). 
>
> Use "local $/ = undef;" for (temporary) slurp mode.
>
> Signed-off-by: Jakub Narebski <jnareb@xxxxxxxxx>

I think this part of the gitweb code predates the git-diff
support by the core level.  If I recall correctly, back then we
did not even have "git-diff-{tree,files,index}" when this part
was written.

I think you could use "git-diff $from $to" instead of using
temporary files and running /usr/bin/diff on it with today's
git.  Which is a big win from both security and ease of
administration point of view.  Not having to write anything into
temporary file from gitweb.cgi means you do not have to worry
about stray gitweb process leaving temporary files dangling
behind, for example.

You obviously would need to emulate the -L$from_name -L$to_name
part by hand in the loop that reads from the diff output.

One worrysome pathological case is what happens if files being
tracked happen to have names that consist of 40-byte hexdigits.
I think "git-diff $from $to --" should do the right thing for
gitweb use ($from and $to should not mistaken taken as pathnames
with the explicit -- disambiguator) and "git-diff -- $from $to"
also should do the right thing to take $from and $to as pathnames
even if there are revisions with the same object names, but
somebody may want to verify that (and send patches to fix if it
is not the case).

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