Re: What's cooking in git.git (Jun 2009, #01; Fri, 12)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano wrote:
> * tr/die_errno (Mon Jun 8 23:02:20 2009 +0200) 4 commits
>  - Use die_errno() instead of die() when checking syscalls
>  - Convert existing die(..., strerror(errno)) to die_errno()
>  - die_errno(): double % in strerror() output just in case
>  - Introduce die_errno() that appends strerror(errno) to die()

I had a look at your

  [526abe63] die_errno(): double % in strerror() output just in case

and it seems it doesn't cover all corner cases:

> @@ -64,8 +64,20 @@ void die_errno(const char *fmt, ...)
>  {
>  	va_list params;
>  	char fmt_with_err[1024];
> -
> -	snprintf(fmt_with_err, sizeof(fmt_with_err), "%s: %s", fmt, strerror(errno));
> +	char str_error[256], *err;
> +	int i, j;
> +
> +	err = strerror(errno);
> +	for (i = j = 0; err[i] && j < sizeof(str_error) - 1; ) {
> +		if ((str_error[j++] = err[i++]) != '%')
> +			continue;

If we copied a '%' here, but filled 'str_error', then

> +		if (j < sizeof(str_error) - 1)
> +			str_error[j++] = '%';
> +		else
> +			break;

we 'break' here, instead of tacking on another '%'.  This subsequently
leaves a single trailing '%' at the end of the string.  A possible fix
would be to use

+		else {
+			j--;
+			break;
+		}

instead, so that the terminator ends up on the second-to-last position
in the string, overwriting the lonely '%'.

> +	}
> +	str_error[j] = 0;
> +	snprintf(fmt_with_err, sizeof(fmt_with_err), "%s: %s", fmt, str_error);
>  
>  	va_start(params, fmt);
>  	die_routine(fmt_with_err, params);


I cannot find an explicit mention that trailing '%'s are bad, and my
printf() just ignores them, but 'man 3p printf' (showing a "POSIX
Programmer's Manual" entry that I can't seem to find on the web...) at
least states

  [after explaining all conversion specifiers]

  If a conversion specification does not match one of the above forms,
  the behavior is undefined. If any argument is not the correct type
  for the corresponding conversion specification, the behavior is
  undefined.


[I still like v2 better because it's far less complicated...]

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]