Jeff King <p...@peff.net> writes: > On Mon, Aug 24, 2015 at 11:15:55AM -0700, Junio C Hamano wrote: > >> > This all looks good to me. The topics-in-flight compatibility stuff in >> > patches 3 and 5 is neatly done. Usually I would just cheat and change >> > the order of arguments to make the compiler notice such problems, but >> > that's hard to do here because of the varargs (you cannot just bump >> > "flags" to the end). >> >> Actually, I think my compatibility stuff is worthless. It would not >> catch new callers that wants to only probe and do their own error >> handling by passing 0 (and besides, assert() is a shoddy way to do >> this---there is no guarantee that tests will trigger all the >> codepaths in the first place). > > Oh, hrm, you're right. I was focused on making sure the common 1-passers > were not broken, but patch 3 does break 0-passers (obviously, because > they needed updated in the same patch ;) ). > > And I do agree that build-time assertions are much better than run-time > ones. > >> We should deprecate and remove write_file() by renaming the one with >> the updated semantics to something else, possibly with a backward >> compatiblity thin wrapper around it that is called write_file(), or >> without it to force a link-time error. > > That sounds reasonable. Maybe "format_to_file" or something?
I am going into a slightly different tangent. Binary support is not something we need right now, so I'll keep the door open for that in the future by - drop the "int fatal" altogether from write_file() without adding "unsigned flags"; - add write_file_gently(), again without "unsigned flags"; - make them call write_file_v() that takes "unsigned flags" and va_list. I earlier said there were 2 oddball callers, one of them being suspicious, but it turns out that there are 3 callers that want write_file_gently(). Only the one in the setup codepath is asking for "fatal=0" while discarding the error return, which is suspicious; other two are handling an error themselves and are OK. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
