Re: [PATCH] Improved error messages when temporary file creation fails

[Jonathan Nieder <jrnieder@gmail.com>]

Hi again,

Arnout Engelen wrote:

> Updated version of the patch

Almost there.  Some remaining nits from my pov:

> , taking into account the feedback from this 
> thread.
[...]

This text above the "---" becomes part of the log message when a patch
is committed to git.git, so it is best to make it self-contained.  The
usual advice is "describe the current behavior, why the proposed
behavior is better, and then how the proposed behavior is achieved."

> --- /dev/null
> +++ b/test-mktemp.c
> @@ -0,0 +1,15 @@
> +/*
> + * test-mktemp.c: code to exercise the creation of temporary files
> + */
> +#include <string.h>
> +#include "git-compat-util.h"

git-compat-util.h takes care of portably including system headers in
the right order.  (For example, #include-ing <string.h> before
setting _POSIX_SOURCE will cause some symbols not to be defined in
_other_ headers on some operating systems, iirc.)  I'd suggest
removing the redundant #include <string.h>.

See Documentation/CodingGuidelines.

> +int main(int argc, char *argv[])
> +{
> +	if (argc != 2)
> +		usage("Expected 1 parameter defining the temporary file template");
> +
> +	xmkstemp(strdup(argv[1]));

Why not xstrdup(), which diagnoses allocation failures?

> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -196,10 +196,22 @@ FILE *xfdopen(int fd, const char *mode)
>  int xmkstemp(char *template)
>  {
>  	int fd;
> +	char origtemplate[PATH_MAX];
> +	strlcpy(origtemplate, template, sizeof(origtemplate));
>  
>  	fd = mkstemp(template);
> -	if (fd < 0)
> -		die_errno("Unable to create temporary file");
> +	if (fd < 0) {
> +		int saved_errno = errno;
> +		const char * nonrelative_template;

It would be more usual not to include a space between the '*' and
'nonrelative_template'.

Thanks for your perseverance.