Arnout Engelen <arnouten@xxxxxxxx> writes:
> This patch has been submitted/discussed before, but that version doesn't apply
> cleanly to the newest git anymore, so here's an updated version.
Thanks, but that comes after "---", no?
> It improves diagnostic error messages when creating a temporary file fails.
>
> Signed-off-by: Arnout Engelen <arnouten@xxxxxxxx>
> ---
> diff --git a/wrapper.c b/wrapper.c
> index 4c1639f..6640c87 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -2,6 +2,7 @@
> * Various trivial helper wrappers around standard functions
> */
> #include "cache.h"
> +#include "wrapper.h"
>
> static void do_nothing(size_t size)
> {
> @@ -196,10 +197,20 @@ FILE *xfdopen(int fd, const char *mode)
> int xmkstemp(char *template)
> {
> int fd;
> + char origtemplate[255];
> + strlcpy(origtemplate, template, 255);
Why "255"?
It may happen to be sufficiently large for the current callers, but what
provisions if any are made to help the compiler or the runtime protect us
from new and broken callers? Use of strlcpy() there hides the issue from
the runtime by avoiding segfault, but it actively harms us by making the
code silently behave incorrectly without segfaulting, no?
> diff --git a/wrapper.h b/wrapper.h
> new file mode 100644
> index 0000000..b06ff0d
> --- /dev/null
> +++ b/wrapper.h
> @@ -0,0 +1,4 @@
> +/*
> + * Various trivial helper wrappers around standard functions
> + */
> +int xmkstemp(char *template);
Somewhat questionable...
Why doesn't this say "extern"?
What happend to another copy of this line in git-compat-util.h? IOW, what
protects this and the other one from drifting apart?
Why doesn't users include wrapper.h but only wrapper.c?
Why yet another header is needed only for this function and nothing else?
Why isn't this protected with the standard #ifndef .../#define .../#endif?
--
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