Re: [PATCH 06/10] diff.c: convert trivial snprintf calls to xsnprintf

[Jeff King]

On Fri, Jun 03, 2016 at 07:47:20AM +, Elia Pinto wrote:

>  diff.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/diff.c b/diff.c
> index d3734d3..fb61539 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4491,7 +4491,7 @@ static int diff_get_patch_id(struct diff_options 
> *options, unsigned char *sha1)
>   len1 = remove_space(p->one->path, strlen(p->one->path));
>   len2 = remove_space(p->two->path, strlen(p->two->path));
>   if (p->one->mode == 0)
> - len1 = snprintf(buffer, sizeof(buffer),
> + len1 = xsnprintf(buffer, sizeof(buffer),
>   "diff--gita/%.*sb/%.*s"
>   "newfilemode%06o"
>   "---/dev/null"

More that assume that PATH_MAX is plenty big for any patch we would
see. I'd argue these should be on the heap, which removes that
restriction, and gets rid of the "PATH_MAX * 4 + 20" magic at the top of
the function.

It looks like the buffer just gets fed into sha1, so I was also tempted
to suggest we simply feed the bits of string in directly. But we are
using the format string here to handle the mode, so you'd still have to
deal with that.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/10] diff.c: convert trivial snprintf calls to xsnprintf

[Elia Pinto]

With the commits f2f02675 and 5096d490 we have been converted in some files the 
call
from snprintf/sprintf/strcpy to xsnprintf. This patch converts the remaining 
calls
to snprintf with xsnprintf under the following conditions:

- The call to snprintf does not control the outcome of the command
  or the presence of truncation errors.
- A call to snprintf can generate a fatal error, directly or indirectly.

The other few remaining cases in which a call to snprintf can generate a soft 
error
have not been changed.

Signed-off-by: Elia Pinto 
---
 diff.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/diff.c b/diff.c
index d3734d3..fb61539 100644
--- a/diff.c
+++ b/diff.c
@@ -4491,7 +4491,7 @@ static int diff_get_patch_id(struct diff_options 
*options, unsigned char *sha1)
len1 = remove_space(p->one->path, strlen(p->one->path));
len2 = remove_space(p->two->path, strlen(p->two->path));
if (p->one->mode == 0)
-   len1 = snprintf(buffer, sizeof(buffer),
+   len1 = xsnprintf(buffer, sizeof(buffer),
"diff--gita/%.*sb/%.*s"
"newfilemode%06o"
"---/dev/null"
@@ -4501,7 +4501,7 @@ static int diff_get_patch_id(struct diff_options 
*options, unsigned char *sha1)
p->two->mode,
len2, p->two->path);
else if (p->two->mode == 0)
-   len1 = snprintf(buffer, sizeof(buffer),
+   len1 = xsnprintf(buffer, sizeof(buffer),
"diff--gita/%.*sb/%.*s"
"deletedfilemode%06o"
"---a/%.*s"
@@ -4511,7 +4511,7 @@ static int diff_get_patch_id(struct diff_options 
*options, unsigned char *sha1)
p->one->mode,
len1, p->one->path);
else
-   len1 = snprintf(buffer, sizeof(buffer),
+   len1 = xsnprintf(buffer, sizeof(buffer),
"diff--gita/%.*sb/%.*s"
"---a/%.*s"
"+++b/%.*s",
-- 
2.9.0.rc1.265.geb5d750

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html