Hi Junio, On 2015-06-21 19:36, Junio C Hamano wrote: > 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.
How about I implement your suggestion tomorrow, then show the diff between the two versions and we can assess what looks to be simpler (i.e. more maintainable)? BTW I agree about the skip-list feature being the most important outcome of this patch series; It only occurred to me as a useful feature at the very end of my work on the fsck IDs, but then I did not want to throw away all of that previous work ;-) Ciao, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in
