On Sat, Dec 31, 2016 at 08:58:43AM +0100, Michael Haggerty wrote: > > The return value is always "0" or "-1". It seems like it would be > > simpler to just return the descriptor instead of 0. > > > > I guess that makes it hard to identify the case when we chose not to > > create a descriptor. I wonder if more "normal" semantics would be: > > > > 1. ret >= 0: file existed or was created, and ret is the descriptor > > > > 2. ret < 0, err is empty: we chose not to create > > > > 3. ret < 0, err is non-empty: a real error > > I don't like making callers read err to find out whether the call was > successful, and I think we've been able to avoid that pattern so far. I guess my mental model is that case 2 _is_ a failure, because we didn't open the reflog. It's just one that callers may want to distinguish from case 3, because it's probably a silent failure, not one we want to complain to the user about. But whether that's accurate would depend on the callers. Looking at the callers, I think the immediate callers would be happier with this, but you probably would want to end up converting case 3 back to "return 0" out of files_log_ref_write(). > > I dunno. This may just be bikeshedding, and I can live with it either > > way (especially because you documented it!). > > Let's see if anybody has a strong opinion about it; otherwise I'd rather > leave it as is. Sounds good. -Peff