Evan Layton wrote: > Jean McCormack wrote: >> be_mount.c: >> line 348: I'm confused as to why you do the be_get_uuid call. uu >> is never used after it and >> it's not clear there's any side effect. So why do so? If >> there's a reason a comment would help. >> >> line 351: Do you need the if(gen_tmp_altroot) free(tmp_altroot); >> code here? >> line 477: ditto >> >> line 545: Aren't we supposed to use strncmp for security reasons? >> >> line 598: ditto >> >> line 645: Shouldn't these really be be_errno_t? >> >> line 679: Nit (there always has to be at least one nit). Remove >> the get, it's already on the line above. >> >> line 828- 841: What happens if the strlcpy actually truncates dir? >> >> line 851: Is this really none? Maybe something here or in the >> returns about tmp_mp. >> >> line 858 & 861: shouldn't this really be be_errno_t? (Actually >> this is going to be a generic comment >> since I see it many places) >> >> line 867: should this be strncpy? >> >> line 940 & 942: should this be strncmp? >> >> line 1447: Huh? >> >> >> line 1895: do we need to free altroot? >> >> line 2175: Maybe file a bug on this? >> >> be_utils.c >> general: same comment as above for the return types. The comment >> says be_errno_t but it returns an int and ret/err are ints. I >> know it's really the same thing but it would be nice to see >> the cleanup done sometime. Maybe a low level bug? >> >> line 1710: continaer_ds -> container_ds > > fixed... > >> >> line 1754: Is there a bug open on this? > > No, running BE management commands is currently not supported within a > zone. > >> >> line 1755: furture -> future >> >> line 2478: containder ->container > > #...@$^$%@ fingers... oh well at least they're consistently wrong ;-) > >> >> cleanup: I believe a close(fd) should be there. > > what line? Sorry. Somewhere in the cleanup that starts at 2664.
Jean > > Thanks! > -evan
