Keep in mind that assert() can be compiled out if NDEBUG is defined. I don't think that this is done very often, however.
I've seen some places in the code where there is an assert() followed by an abort(). tom Steve Peng writes: > > David, > > Ok I will go ahead assert in backend_switch_copy(). I will send out > another increment webrev once it is ready. > > Thanks > > Steve > > David Bustos wrote: > > Quoth Steve Peng on Thu, Feb 14, 2008 at 10:58:14AM -0700: > > > >> David Bustos wrote: > >> > >>> backend_switch() no longer fails with _TRUNCATED, so please remove it > >>> from repository_switch()'s comment. > >>> > >> Line 1172 in backend_switch_copy possibly can return _TRUNCATED error > >> hence backend_switch can return the same > >> error. > >> > > > > Not if you assert() that backend_switch_copy() doesn't fail with > > _TRUNCATED. If it does, we'll abort and the caller of backend_switch() > > will never see the error. > > > > > >>>>> cmd/svc/configd/backend.c > >>>>> > >>> ... > >>> > >>>>> 1302: Shouldn't you assert() that this doesn't fail with _TRUNCATED > >>>>> since you know that dst is much shorter than PATH_MAX? > >>>>> > >>>>> > >>>> I can add that. > >>>> > >>> Then you should also fix backend_switch()'s comment to say that it can > >>> return the error codes from backend_switch_copy() except for _TRUNCATED. > >>> Though I think it would be better to just list the errors outright. > >>> > >> Look into more I think I should back out assertion since strcat in > >> backend_switch_copy can possibly fail with _TRUNCATE even > >> it may not happen but I like to be more cautious. > >> > > > > No, we can prove that it can't happen. It doesn't depend on the value > > of sw_back, it depends on REPOSITORY_DB, FAST_REPOSITORY_DB, PATH_MAX, > > and the code of backend_switch_copy(), all of which are either standard > > or are compiled into the binary. It means that somebody changed one of > > those things in an uncorrelated way and is testing it. Rather than > > passing an error up the stack which eventually results in a strange > > error message, you should abort() as soon as we know it's a programming > > error. (Some would even argue that you could assert() when strlcat() > > fails in backend_switch_copy().) This will make debugging easier and > > eliminates an error case for the calling code. > > > > If you want to make the assumptions more explicit, you could calculate > > how long dst has to be before backend_switch_copy() returns _TRUNCATED > > and document it in the comment. Then you could add > > assert(strlen(dst) < /* computed number */) to backend_switch(). And > > you could even add a comment to the definition of REPOSITORY_DB and > > FAST_REPOSITORY_DB saying that it shouldn't be made any longer than your > > computed number. I don't think it's worth it for such specialized code > > in the same file, though. > > > > > > David > > > > _______________________________________________ > smf-discuss mailing list > smf-discuss at opensolaris.org
