On Wed, Feb 16, 2011 at 12:23 PM, M. Mohan Kumar <mo...@in.ibm.com> wrote: > +/* > + * Returns file descriptor of dirname(path) > + * This fd can be used by *at functions > + */ > +static int get_dirfd(FsContext *fs_ctx, const char *path) > +{ > + V9fsFileObjectRequest request; > + int fd, error = 0; > + char *dpath = qemu_strdup(path); > + > + fill_request(&request, dirname(dpath), NULL); > + request.data.type = T_OPEN; > + fd = v9fs_request(fs_ctx, &request, &error); > + if (fd < 0) { > + errno = error; > + }
man errno says: "a function that succeeds is allowed to change errno" Therefore you can't safely use errno to stash your error value and call any code outside your control because it may overwrite errno. Obliging callers to take these precautions is a bad idea because they will forget. The main feedback I have for this entire patchset is that these functions introduce layers and layers of weird error handling. A large fraction of the code is just propagating error codes. Sometimes errno is used, sometimes an int * argument is used, sometime a positive errno is returned although QEMU code normally returns a negative errno. Then the fd_info struct has a flag to indicate the fd is invalid when we could just set it to -1 to indicate this. It's messy and hard for someone else to modify later without introducing error handling bugs. The method that fits most with QEMU's codebase is: int foo(...) /* returns 0 on success, -errno on failure */ If you can refactor the code to unify error handling it will require fewer lines and be simpler. Stefan