Johannes Schindelin <johannes.schinde...@gmx.de> writes: > Hmm. I really do not like that kind of thinking, i.e. having to > duplicate, then modify data to be able to call the API, only to have > to modify the data back afterwards, and eventually having to > unallocate the data in all code paths. That feels just very inelegant > to me.
You can see in our codebase that I have avoided touching end-user strings by using a substring in place by introducing a new API function or using an existing API function that takes <ptr, len> for exactly that reason. There also are cases where we are better off if we make a copy upfront at a very high level in the callchain if that makes the processing of that string deeper in the callchain much simpler without customized helpers that take counted strings. And 03/19 and this one taken together, I think it is an example of the latter [*1*]. You not only need to invent counted string comparison in 04/19 but also need upcasing byte-by-byte comparison in a loop in 03/19; both of which can be made much simpler if you massaged the end-user input "foo=error,bar=ignore" into "FOO=error,BAR=ignore" and allowed the code to loop over it to turn ',' into NUL while parsing each individual piece (i.e. "FOO=error"). So contrary to what you said in response to my review on 03/19, I view this as not "adding complexity" but its total opposite. It is to make the code and logic much less complex by paying the price for one copied (and massaged) string. Having said all that, as long as the result functions correctly, I suspect that it may not matter much either way. As I already said, "we can demote this error to a warning (or ignored)" is much less useful in practice than the "we know this and that object in this project does not pass fsck, so please do not bother checking" in my mind, and that makes me think that this part of the series is much less important than the skip-list thing. Unnecessary complexity in the code and otherwise useless helper functions may be something that can be simplified following my advice, but that can be done as a code clean-up, simplification and optimization by somebody else later if they really cared. Thanks. [Footnote] *1* Everything I say in my messages is what "I think", so saying "I think" before saying what I think is redundant, but this one needs that, because I think ;-) it is in the "taste" territory to view each individual case and decide which one of these two opposite approaches is more appropriate. -- To unsubscribe from this list: send the line "unsubscribe git" in
