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