[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [V5 PATCH 8/8] virtio-9p: Chroot environment for other
From: |
Stefan Hajnoczi |
Subject: |
[Qemu-devel] Re: [V5 PATCH 8/8] virtio-9p: Chroot environment for other functions |
Date: |
Thu, 17 Feb 2011 11:02:03 +0000 |
On Wed, Feb 16, 2011 at 12:23 PM, M. Mohan Kumar <address@hidden> 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
- [Qemu-devel] [V5 PATCH 0/8] virtio-9p: Use chroot to safely access files in passthrough security model, M. Mohan Kumar, 2011/02/16
- [Qemu-devel] [V5 PATCH 7/8] virtio-9p: Move file post creation changes to none security model, M. Mohan Kumar, 2011/02/16
- [Qemu-devel] [V5 PATCH 5/8] virtio-9p: Create support in chroot environment, M. Mohan Kumar, 2011/02/16
- [Qemu-devel] [V5 PATCH 3/8] virtio-9p: Add client side interfaces for chroot environment, M. Mohan Kumar, 2011/02/16
- [Qemu-devel] [V5 PATCH 8/8] virtio-9p: Chroot environment for other functions, M. Mohan Kumar, 2011/02/16
- [Qemu-devel] Re: [V5 PATCH 8/8] virtio-9p: Chroot environment for other functions,
Stefan Hajnoczi <=
- [Qemu-devel] [V5 PATCH 2/8] virtio-9p: Provide chroot environment server side interfaces, M. Mohan Kumar, 2011/02/16
- [Qemu-devel] [V5 PATCH 4/8] virtio-9p: Add support to open a file in chroot environment, M. Mohan Kumar, 2011/02/16
- [Qemu-devel] [V5 PATCH 6/8] virtio-9p: Support for creating special files, M. Mohan Kumar, 2011/02/16
- [Qemu-devel] [V5 PATCH 1/8] Implement qemu_read_full, M. Mohan Kumar, 2011/02/16