From: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date: Mon, 09 Nov 2009 19:16:15 GMT
Subject: Re: [PATCH] git_odb_open ckeck for valid path to database
Message-ID: <4AF86A7F.1010500@ramsay1.demon.co.uk>
In-Reply-To: <1257510729-6803-1-git-send-email-kde@mosehansen.dk>

Esben Mose Hansen wrote:
> diff --git a/src/odb.c b/src/odb.c
> index 6d646a4..89d6d03 100644
> --- a/src/odb.c
> +++ b/src/odb.c
> @@ -30,6 +30,10 @@
>  #include "hash.h"
>  #include "odb.h"
>  
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <unistd.h>
> +

None of these #includes should be necessary (they are not for me).
Also, "#include <unistd.h>" breaks on windows ;-)

>  #define GIT_PACK_NAME_MAX (5 + 40 + 1)
>  struct git_pack {
>  	git_odb *db;
> @@ -1023,6 +1027,15 @@ int git_odb_open(git_odb **out, const char *objects_dir)
>  		free(db);
>  		return GIT_ERROR;
>  	}
> +	struct stat objects_dir_stat;

This should be declared at the head of the function (or generally at the start of
a block/scope); ie we don't use this C99 facility. This breaks MSVC, which does
not understand C99.

> +	if (stat(db->objects_dir, &objects_dir_stat)) {
> +		free(db);
> +		return GIT_EOSERR;
> +	}
> +	if (!S_ISDIR(objects_dir_stat.st_mode)) {

This breaks on MSVC, since the MS headers do not define S_ISDIR. It's not difficult
to add; it's on my TODO list. (Andreas, I can add this later if you like)

However, I suspect it may be useful to add an gitfo_is_directory() function, or
something like it. Maybe. Andreas?

Thank you for the patch.

(I think Andreas would prefer to see libgit2 somewhere in the subject, perhaps
like [LIBGIT2 PATCH], otherwise he may miss you email)

ATB,
Ramsay Jones