--On Thursday, February 26, 2009 10:26:05 AM -0800 "John Zolnowsky x69422/408-404-5064" <John.Zolnowsky at sun.com> wrote:
> >> From: Gary Winiger <gww at eng.sun.com> >> > Bart Blanquart wrote: >> > > On 02/25/09 22:09, Scott Rotondo wrote: >> > > > Bart Blanquart wrote: >> > > >> Current webrev at >> > > >> http://cr.opensolaris.org/~bartbl/6251549-20090225/ >> > > > >> > > > This is OK, but it would also work fine if you get rid of >> > > > strdup() and the corresponding free() at line 136. >> > > >> > > Yeah, but that makes things more fragile. A later call to getpw*() >> > > could change the username being processed. >> > >> > Since we know the only such calls in this program are in >> > get_username(), I would get rid of the strdup(). However, I don't >> > feel strongly enough to insist on this point. >> >> I feel a lot stronger than Scott on this point. IMO, it is misleading >> to future maintainers. It also overly complicates the code, again >> misleading future maintainers. > > I'd take this as a matter of style. To me, the interface as defined is > much simpler for future maintainers: rather than returning a static > value which the caller is responsible for protecting against calls to > getpw*(), it returns a string which the caller is responsible for > deallocating. The latter model strikes me as simpler to describe and > enforce. getpw*() is an abomination, like all interfaces which depend on returning pointers to static internal buffers that may change unexpectedly in the future. Depending on such internal buffers to remain unchanged is either almost as bad or quite a bit worse, depending mostly on whether or not one has already been bitten by the particular usage in question. Keep the strdup.
