Re: read_branches_file ()

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Feb 8, 2008, 16:22 UTC
Message-ID: <alpine.LSU.1.00.0802081618210.11591@racer.site>
In-Reply-To: <20080208170305.069d43d2@pc09.procura.nl>

Hi,

On Fri, 8 Feb 2008, H.Merijn Brand wrote:

Show 8 quoted lines

> --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.

Show 6 quoted lines

> @@ -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".

Also, we only leave a space after operators like "for", "while", but not after function names.

> +       if (stat (gp, &st_buf) || S_ISDIR (st_buf.st_mode))

Again, please remove the spaces after "stat" and "S_ISDIR".

Other than that, the patch looks obviously correct: please resubmit with a nice commit message and a sign-off.

Thanks, Dscho