> I see that now. That said, your version now does this for _every_ caller > of e2p_feature2string(), while the original code only did it in the rare > case of an unsupported feature bit set on on a filesystem. I don't see > that as an improvement over the original. I do - the existing code is causing data corruption in my use. I'm unsure why the string copy is problematic - this doesn't seem to be performance-sensitive code. As far as I could find, all uses of this function in the e2fsprogs codebase are just printfing the result, in human-readable format. Or for debugging. ? > One option, maybe not very pretty but not fatally ugly, is to just > initialize all of the "unknown" features as static strings. That ends > up being 23 extra "FEATURE_Cnn" strings, 21 extra "FEATURE_Rnn" strings, > and 17 extra "FEATURE_Inn" strings. > > That ends up being less than 1KB of static text, which isn't bad at all. > > This could potentially speed up e2p_feature2string(), since it could > index directly into three different arrays instead of having to scan all > of them to find the right f_compat type first. I'm not sure if it really > makes a noticeable difference in the end though. I can do either, if you insist. What would you prefer: a) Statically defining the various FEATURE_foo strings, as you describe. b) Duplicating the implementation of the two functions so that e2p_feature2string() can retain its prior behaviour of returning a string constant in the common cases. Or alternatively, I suppose: c) Use a thread-local internal buffer to return the results, so it's at least thread-safe (though still not really safe, since multiple calls on one line - e.g. as printf arguments - could still evaluate to incorrect results). -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
