Re: [PATCH] system_path: use a static buffer

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

 



On Mon, Mar 21, 2011 at 10:56:19AM +0100, Carlos MartÃn Nieto wrote:

> On vie, 2011-03-18 at 00:25 -0700, Junio C Hamano wrote:
> > Carlos MartÃn Nieto <cmn@xxxxxxxx> writes:
> > 
> > > +	ret = snprintf(buf, sizeof(buf), "%s/%s", prefix, path);
> > > +	if (ret >= sizeof(buf))
> > > +		die("system path too long for %s", path);
> > > +	else if (ret < 0)
> > > +		die_errno("encoding error");
> > 
> > POSIX says snprintf() should set errno in this case, and your use of
> > die_errno() would show that information, but what is "encoding error"?
> > 
> > Just being curious, as I suspect that "snprintf() returned an error" may
> > be more appropriate, if the answer is "I don't know what kind of error it
> > is, but snprintf() found something faulty while encoding so I chose to
> > call it encoding error".
> 
>  My manpage says snprintf returns -1 if there was an output or encoding
> error. As there couldn't be an output error because it's writing to
> memory and we can't output what snprintf chocked on because whatever
> die_errno uses will also choke on it, I just put "encoding error". I'd
> put "error assembling system path" as the actual error message, I guess.

FWIW, we don't catch snprintf failures in 99% of the calls in git. Most
calls just ignore the return value, and some even directly use the
return value to add to a length. The one place that actually does check
for the error is strbuf_vaddf, which just says "your vsnprintf is
broken" and dies.

So I'm not sure how much we really care about this error code path. If
anything, we should be replacing all of the calls with something like:

  static const char buggy_sprintf_msg[] =
  "BUG: vsnprintf returned %d; either we fed it a bogus format string\n"
  "(our bug) or your libc is buggy and returns an error when it should\n"
  "tell us how much space is needed. The format string was:\n"
  "%s\n";
  int xsnprintf(char *out, size_t size, const char *fmt, ...)
  {
          va_list ap;
          int r;

          va_start(ap, fmt);
          r = vsnprintf(out, size, fmt, ap);
          va_end(ap);

          if (r < 0)
                  die(buggy_sprintf_msg, r, fmt);
          return r;
  }

-Peff
--
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]