On May 14, 2012, at 1:57 PM, Gregory Farnum wrote: > On Mon, May 14, 2012 at 10:49 AM, Gregory Farnum <greg@xxxxxxxxxxx> wrote: >> I haven't had the chance to go over everything thoroughly (I notice a >> few other users of EDOM that I want to check out), but those >> definitely sound better to me. >> More better review comments later today. >> -Greg > > After reviewing, I do have one other note, and that's on the use of > errno. Most of Ceph avoids errno; it's only set in 19 places in the > code and a lot of those don't count. However, we seem to have run into > a need for it (readdir), and this patchset has both rewinddir and > seekdir returning under error conditions without doing any > notification. So I think both of those functions (and any I missed) > need to set errno if they bail out — the other possibility is > converting them to have a return code, which I'm not opposed to but is > another user-visible change. In contrast to readdir, the seek/rewinddir man pages state that these functions do not return an error (via errno or otherwise). So, presumably a client assuming man-page-ish semantics would never check errno after seekdir, which could have odd implications for arbitrary code just trying to tie into libcephfs. Then again, maybe they should use libcephfs man pages :) ? - Noah-- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
