Subject: Re: CVS commit: src/libexec/comsat
To: None <itojun@iijlab.net>
From: Valeriy E. Ushakov <uwe@ptc.spbu.ru>
List: source-changes
Date: 09/19/2003 18:03:08
On Fri, Sep 19, 2003 at 15:03:05 +0900, itojun@iijlab.net wrote:
> >I'm not sure I understand what actually changed here. The old code did:
> >
> > newsize = some expression;
> > buf = realloc(newsize);
> > if (buf == NULL)
> > exit();
> >
> >Now we say:
> >
> > tmp buf = realloc(some expression);
> > if (tmp buf == NULL)
> > exit();
> > buf = tmp buf;
> > newsize = some expression;
> >
> >It seems to me that we've introduced extra complexity for absolutely no
> >reason at all. Because the program exits immediately in the failure
> >case (without jumping through a pile of clean-up functions), there's
> >no chance that an invalid buffer or buffersize will be used in further
> >code.
>
> if we leave bad practice in code people would cut-and-paste it and use
> it, causing bad mistakes. so i am being pedantic.
And if I read this right, then, *unlike* the ssh buffer.c bug, in this
case if realloc fails the buf will be NULL. And as Simon pointed out
the code calls exit() immediately, anyway.
So it seems that you are copy-pasting the ssh buffer.c fix without
checking if it's applicable or not, effectively making the very
mistake you are claiming you want to prevent from happening.
SY, Uwe
--
uwe@ptc.spbu.ru | Zu Grunde kommen
http://www.ptc.spbu.ru/~uwe/ | Ist zu Grunde gehen