Olaf Hering wrote: > On Mon, Dec 01, Kevin J. McCarthy wrote: > > > In general, mutt checks NULLs pretty well, but returning NULL from > > mutt_substrdup() isn't without risk of just generating a segfault in > > another place. So personally I would vote for the second or third > > choice. > > Why is there any risk? Just audit the code. mutt source is just a few > files and mutt is not a library with external callers...
Yes, precisely: mutt_substrdup is not an external library, it's a *part* of mutt. The current contract for mutt_substrdup is that (begin != NULL) , begin <= end, and that the return value will be != NULL. Violations of this contract generally should be fixed at the caller level. The current patches do just that, and it may be perfectly reasonable to leave it at that. However, this exact same error has occurred in the past, so it's also reasonable to ask if mutt_substrdup should check for violations of this contract. The question is what should it do if it detects what is basically an internal logic error in mutt? If we return NULL, should we audit all the callers and add NONULL around each usage of the return value? Even for cases where the return value should "never" be NULL? (e.g. init.c line 245). Essentially, this is "papering over" the logic error, so why not just set len=0 instead? If we're not going to add NONULL checks, why not just blow up inside mutt_substrdup instead? -Kevin
signature.asc
Description: PGP signature
