David, Comments inline.
Thanks - Steve David Bustos wrote: > Quoth Steve Peng on Thu, Feb 14, 2008 at 09:33:14AM -0700: > >>> cmd/svc/configd/client.c >>> 2008: According to the comment before backend_switch() in backend.c, >>> this function can also fail with _BACKEND_READONLY and _TRUNCATED. >>> >> Added. >> > > 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. > >>> 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. > >>> 1317: What happens if this fails? >>> >> fail the operation. >> > > Shouldn't you also free(be->path) before you overwrite it? And > shouldn't you sqlite_close(new) if the strdup() fails? Why are you > returning _BACKEND_ACCESS? Isn't _NO_RESOURCES more appropriate? > I think you are right. I should sqlite_close new and free the previous strdup. > >>> lib/libscf/inc/libscf_priv.h >>> 315-20: Please move these up with the other #defines. >>> >> Done. >> > > 283: Blank line, please. > > 286-7: Can these be aligned with line 288? > Sure. > >>> lib/libscf/common/lowlevel.c >>> > ... > >>> 6874: According to the comment before repository_switch() in client.c, >>> this request can also fail with _BACKEND_ACCESS and _NO_RESOURCES. >>> >> Added. >> > > For _NO_RESOURCES, you should specify that svc.configd is out of memory. > As you wish. > > David >
