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
