Michael McConville wrote:
> Nicholas Marriott wrote:
> > Looks good, ok nicm
>
> Reviewing now, generally looks good.
>
> A few things:
>
> I don't understand the motive for all the err() -> errx() and fatal() ->
> fatalx() changes. If I came across these, I probably would have
> suggested the reverse. err(1, "xstrdup") is a lot cleaner than a long
> custom error message, IMO. I don't know how much value is in showing the
> size of the failed allocation, either - thoughts on that? I'm fine with
> a little less uniformity for simplicity's sake.
Showing the size lets you determine if something has gone terribly
wrong (can't allocate 3840284093284092840928402 bytes) versus a more mundane
out of memory condition. Allocation failure is usually the final symptom of
some other disease.
But agreed that always printing "out of memory" seems like a step backwards
from printing what errno tells us. It's discarding information.
> Also, I'm seeing a couple "could not allocate memory" messages added to
> *snprintf() functions. They write to a supplied buffer, no?
Good catch.
> > > + i = vsnprintf(str, len, fmt, ap);
> > > va_end(ap);
> > >
> > > - if (i == -1 || i >= (int)size)
> > > - fatal("xsnprintf: overflow");
> > > + if (i < 0 || i >= (int)len)
> > > + fatal("xsnprintf: could not allocate memory");
This change (among a few others) is wrong.
