On Fri, 8 Feb 2008 16:22:45 +0000 (GMT), Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote: > Hi, > > On Fri, 8 Feb 2008, H.Merijn Brand wrote: > > > --8<--- remote.c.diff > > --- remote.c.org 2008-01-27 09:04:18 +0100 > > +++ remote.c 2008-02-08 17:01:09 +0100 > > @@ -1,6 +1,7 @@ > > #include "cache.h" > > #include "remote.h" > > #include "refs.h" > > +#include <sys/stat.h> > > This should not be necessary; we include all system headers in cache.h. dropped > > @@ -173,11 +174,15 @@ static void read_branches_file(struct re > > char *frag; > > char *branch; > > int n = slash ? slash - remote->name : 1000; > > - FILE *f = fopen(git_path("branches/%.*s", n, remote->name), "r"); > > + char *gp = git_path ("branches/%.*s", n, remote->name); > > Please use a more descriptive variable name, such as "branches_file" or > "branches_path". took another approach, as that also addresses the other fopen () call > Also, we only leave a space after operators like "for", "while", but not > after function names. This patch was not sent to be applied as-is, only as a proof-of-concept Not that I agree to the layout/indentation, the new patch is sent trying to follow what you use. > > + if (stat (gp, &st_buf) || S_ISDIR (st_buf.st_mode)) > > Again, please remove the spaces after "stat" and "S_ISDIR". you said please :) > Other than that, the patch looks obviously correct: please resubmit with a > nice commit message and a sign-off. Done -- H.Merijn Brand Amsterdam Perl Mongers (http://amsterdam.pm.org/) using & porting perl 5.6.2, 5.8.x, 5.10.x on HP-UX 10.20, 11.00, 11.11, & 11.23, SuSE 10.1 & 10.2, AIX 5.2, and Cygwin. http://qa.perl.org http://mirrors.develooper.com/hpux/ http://www.test-smoke.org http://www.goldmark.org/jeff/stupid-disclaimers/ - 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