Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

On Thu, Nov 12, 2015 at 02:52:01PM +0800, Michael W. Bombardieri wrote: > > > ok for removing xfree from aucat? > > > > > > > yes, ok ratchov; if later this causes me merges i'll find another > > solution. Feel free to do the same in usr.bin/sndiod, as it's > > almost the same. > > > > Same

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

Alexandre Ratchov wrote: > On Thu, Nov 12, 2015 at 02:52:01PM +0800, Michael W. Bombardieri wrote: > > > > ok for removing xfree from aucat? > > > > > > yes, ok ratchov; if later this causes me merges i'll find another > > > solution. Feel free to do the same in usr.bin/sndiod, as it's > > >

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

Michael W. Bombardieri wrote: > > > ok for removing xfree from aucat? > > > > yes, ok ratchov; if later this causes me merges i'll find another > > solution. Feel free to do the same in usr.bin/sndiod, as it's > > almost the same. > > Same thing for sndiod... ok mmcc@ > Index: abuf.c >

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

> > ok for removing xfree from aucat? > > > > yes, ok ratchov; if later this causes me merges i'll find another > solution. Feel free to do the same in usr.bin/sndiod, as it's > almost the same. > Same thing for sndiod... Index: abuf.c

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

> On November 9, 2015 at 5:04 AM Michael McConville wrote: > Tobias, could you split your latest diff into separate diffs for each > function type (xmalloc, xcalloc, etc.)? It'd make it easier to zero in > on the problematic hunks and fast-track the rest. I don't really see

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

On Sun, Nov 08, 2015 at 07:43:10PM -0500, Michael McConville wrote: > Maybe we should pick this off in smaller chunks so that we don't get > immobilized by a few scattered issues. > > ok for removing xfree from aucat? > yes, ok ratchov; if later this causes me merges i'll find another solution.

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

Hmm yes you are right, in that case, go for it. On Sun, Nov 08, 2015 at 02:00:17PM +0100, Joerg Sonnenberger wrote: > On Sun, Nov 08, 2015 at 08:36:52AM +, Nicholas Marriott wrote: > > On Sat, Nov 07, 2015 at 08:42:14PM -0500, Ted Unangst wrote: > > > Tobias Stoeckmann wrote: > > > > Is this

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

On Sun, Nov 08, 2015 at 08:36:52AM +, Nicholas Marriott wrote: > On Sat, Nov 07, 2015 at 08:42:14PM -0500, Ted Unangst wrote: > > Tobias Stoeckmann wrote: > > > Is this okay for ssh and tmux, which are out to be very portable? > > > Nicholas mentioned that malloc is not required to set errno.

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

Alexandre Ratchov wrote: > On Sun, Nov 08, 2015 at 09:56:23AM +0800, Michael W. Bombardieri wrote: > > On Thu, Nov 05, 2015 at 03:50:29PM +0100, Tobias Stoeckmann wrote: > > > On Thu, Nov 05, 2015 at 09:50:48AM +, Nicholas Marriott wrote: > > > > I don't know why cvs and rcs xmalloc.c has

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

Maybe we should pick this off in smaller chunks so that we don't get immobilized by a few scattered issues. ok for removing xfree from aucat? Index: abuf.c === RCS file: /cvs/src/usr.bin/aucat/abuf.c,v retrieving revision 1.26 diff

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

Michael McConville wrote: > Maybe we should pick this off in smaller chunks so that we don't get > immobilized by a few scattered issues. > > ok for removing xfree from aucat? I just realized that this one was already submitted separately. Tobias, could you split your latest diff into separate

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

This is fine with me, I think it was better without errno, but using it can't do any harm. It is an extension TO set it, not to not set it, but I am pretty sure it only happens on platforms I don't care about :-). I suggest you check with djm or dtucker for ssh in case they do care, or there are

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

On Sat, Nov 07, 2015 at 08:42:14PM -0500, Ted Unangst wrote: > Tobias Stoeckmann wrote: > > Is this okay for ssh and tmux, which are out to be very portable? > > Nicholas mentioned that malloc is not required to set errno. I've also > > checked the standard and it's just an extension. Although at

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

Hi On Sat, Nov 07, 2015 at 04:39:09PM -0500, 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

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

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

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

Here's an updated diff: - use "overflow" error message for snprintf and friends - use err instead of errx for out of memory conditions - if fatal() doesn't print error string, use ": %s", strerror(errno) Is this okay for ssh and tmux, which are out to be very portable? Nicholas mentioned that

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

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

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

On Sat, Nov 07, 2015 at 05:57:55PM -0500, Ted Unangst wrote: > > Also, I'm seeing a couple "could not allocate memory" messages added to > > *snprintf() functions. They write to a supplied buffer, no? > > Good catch. Will update that one, thanks. > > > > + i = vsnprintf(str, len, fmt,

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

Tobias Stoeckmann wrote: > > > > > + i = vsnprintf(str, len, fmt, ap); > > > > > va_end(ap); > > > > > > > > > > - if (i == -1 || i >= (int)size) > > > > > - fatal("xsnprintf: overflow"); > > > > > + if (i < 0 || i >= (int)len) > > > > > +

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

Tobias Stoeckmann wrote: > Is this okay for ssh and tmux, which are out to be very portable? > Nicholas mentioned that malloc is not required to set errno. I've also > checked the standard and it's just an extension. Although at worst, > the user sees a wrong error message... Are they portable to

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

On Thu, Nov 05, 2015 at 03:50:29PM +0100, Tobias Stoeckmann wrote: > On Thu, Nov 05, 2015 at 09:50:48AM +, Nicholas Marriott wrote: > > I don't know why cvs and rcs xmalloc.c has ended up so different. > > It's not just about cvs and rcs: > > /usr/src/usr.bin/cvs/xmalloc.c >

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

Michael W. Bombardieri wrote: > On Thu, Nov 05, 2015 at 03:50:29PM +0100, Tobias Stoeckmann wrote: > > On Thu, Nov 05, 2015 at 09:50:48AM +, Nicholas Marriott wrote: > > > I don't know why cvs and rcs xmalloc.c has ended up so different. > > > > It's not just about cvs and rcs: > > > >

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

I like this a lot. There are some trivial differences in the various xmalloc.h as well, and I think you could make the style consistent within the files (eg "return i" in xasprintf and xsnprintf). On Thu, Nov 05, 2015 at 03:50:29PM +0100, Tobias Stoeckmann wrote: > On Thu, Nov 05, 2015 at

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

On Thu, Nov 05, 2015 at 03:57:26PM +, Nicholas Marriott wrote: > I like this a lot. > > There are some trivial differences in the various xmalloc.h as well, and > I think you could make the style consistent within the files (eg "return > i" in xasprintf and xsnprintf). Oh yes, forgot to

Re: unify xmalloc (was Re: [patch] cvs: retire xfree())

Looks good, ok nicm On Thu, Nov 05, 2015 at 05:35:22PM +0100, Tobias Stoeckmann wrote: > On Thu, Nov 05, 2015 at 03:57:26PM +, Nicholas Marriott wrote: > > I like this a lot. > > > > There are some trivial differences in the various xmalloc.h as well, and > > I think you could make the