On Tue, 10 Sep 2013, Chen Gang wrote: > > I think it would be better to keep mpol_to_str() returning void, and hence > > avoiding the need for this patch, and make it so it cannot fail. If the > > mode is invalid, just store a 0 to the buffer (or "unknown"); and if > > maxlen isn't large enough, make it a compile-time error (let's avoid > > trying to be fancy and allocating less than 64 bytes on the stack if a > > given context is known to have short mempolicy strings). > > > > Hmm... at least, like most of print functions, it need return a value > to tell the length it writes, so in my opinion, I still suggest it can > return a value. > Why? It can just store the string into the buffer pointed to by the char *buffer and terminate it appropriately while taking care that it doesn't exceed maxlen. Why does the caller need to know the number of bytes written? If it really does, you could just do strlen(buffer). If there's a real reason for it, then that's fine, I just think it can be made to always succeed and never return < 0. (And why is nobody checking the return value today if it's so necessary?) > For common printing functions, caller knows about the string format and > all parameters, and also can control them, so for callee, it is not > 'quite polite' to return any failures to caller. :-) > > But for our function, caller may not know about the string format and > parameters' details, so callee has duty to check and process them: > > e.g. "if related parameter is invalid, it is neccessary to notifiy to caller". > Nobody is using mpol_to_str() to determine if a mempolicy mode is valid :) If the struct mempolicy really has a bad mode, then just store "unknown" or store a 0. If maxlen is insufficient for the longest possible string stored by mpol_to_str(), then it should be a compile-time error. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>