On Fri, Sep 09, 2016 at 04:34:47PM -0400, Jeff King wrote: > This patch defines the patch-id of a merge commit as > essentially "null"; it has no patch-id. As a result, > merges cannot match patch-ids via "--cherry-pick", and > "format-patch --base" will not list merges in its list of > prerequisite patch ids. > > To distinguish between real errors and "null", we have to > expand the semantics of commit_patch_id()'s return value, > and callers need to distinguish these cases. One alternative would be to add an out-parameter that is set in the success case saying "yes, we have a real patch-id". And then the callers could look like: if (commit_patch_id(commit, &diffopt, sha1, 0, &got_one)) die("error!"); if (!got_one) continue; /* silently skip */ We could even use the null sha1 to signal that rather than an extra parameter, I suppose. I dunno. It would make the callers less clunky, I think, but it does feel a bit magical. -Peff