Add a new, shared helper function check_name() that consolidates the name validation logic (illegal name check and "." / ".." rejection) currently spread over multiple 9p handlers, unnecessarily duplicating code.
This is pure refactoring with no behavior change. The existing error code semantics are preserved: rename operations return -EISDIR, create operations return -EEXIST. Note: These current error codes actually differ from native Linux system calls (e.g. rename() returns -EBUSY, open(O_CREAT) returns -EISDIR). The 9P protocol does not mandate specific error codes for these validation errors. Hence consolidating to a single error code (e.g., -EINVAL) for all cases could be considered in the future for simplicity reason. This change reduces code duplication across 9 functions: - v9fs_lcreate - v9fs_create - v9fs_symlink - v9fs_link - v9fs_rename - v9fs_renameat - v9fs_wstat - v9fs_mknod - v9fs_mkdir Signed-off-by: Christian Schoenebeck <[email protected]> --- hw/9pfs/9p.c | 100 ++++++++++++++++++++------------------------------- 1 file changed, 39 insertions(+), 61 deletions(-) diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index a09f99537d..9b46b33ba8 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -1810,6 +1810,25 @@ static bool name_is_illegal(const char *name) return !*name || strchr(name, '/') != NULL; } +static int check_name(const char *name, V9fsPDU *pdu) +{ + int request_type = pdu->id; + + if (name_is_illegal(name)) { + return -ENOENT; + } + if (!strcmp(name, ".") || !strcmp(name, "..")) { + /* + * TODO: The different error codes here are just there to preserve + * pre-existing behaviour of 9p server. In future it might make sense to + * consolidate this and e.g. just return -EINVAL for everyone. + */ + return (request_type == P9_TRENAME || request_type == P9_TRENAMEAT || + request_type == P9_TWSTAT) ? -EISDIR : -EEXIST; + } + return 0; +} + static bool same_stat_id(const struct stat *a, const struct stat *b) { return a->st_dev == b->st_dev && a->st_ino == b->st_ino; @@ -2160,13 +2179,8 @@ static void coroutine_fn v9fs_lcreate(void *opaque) } trace_v9fs_lcreate(pdu->tag, pdu->id, dfid, flags, mode, gid); - if (name_is_illegal(name.data)) { - err = -ENOENT; - goto out_nofid; - } - - if (!strcmp(".", name.data) || !strcmp("..", name.data)) { - err = -EEXIST; + err = check_name(name.data, pdu); + if (err < 0) { goto out_nofid; } @@ -2848,13 +2862,8 @@ static void coroutine_fn v9fs_create(void *opaque) } trace_v9fs_create(pdu->tag, pdu->id, fid, name.data, perm, mode); - if (name_is_illegal(name.data)) { - err = -ENOENT; - goto out_nofid; - } - - if (!strcmp(".", name.data) || !strcmp("..", name.data)) { - err = -EEXIST; + err = check_name(name.data, pdu); + if (err < 0) { goto out_nofid; } @@ -3042,13 +3051,8 @@ static void coroutine_fn v9fs_symlink(void *opaque) } trace_v9fs_symlink(pdu->tag, pdu->id, dfid, name.data, symname.data, gid); - if (name_is_illegal(name.data)) { - err = -ENOENT; - goto out_nofid; - } - - if (!strcmp(".", name.data) || !strcmp("..", name.data)) { - err = -EEXIST; + err = check_name(name.data, pdu); + if (err < 0) { goto out_nofid; } @@ -3135,13 +3139,8 @@ static void coroutine_fn v9fs_link(void *opaque) } trace_v9fs_link(pdu->tag, pdu->id, dfid, oldfid, name.data); - if (name_is_illegal(name.data)) { - err = -ENOENT; - goto out_nofid; - } - - if (!strcmp(".", name.data) || !strcmp("..", name.data)) { - err = -EEXIST; + err = check_name(name.data, pdu); + if (err < 0) { goto out_nofid; } @@ -3367,13 +3366,8 @@ static void coroutine_fn v9fs_rename(void *opaque) goto out_nofid; } - if (name_is_illegal(name.data)) { - err = -ENOENT; - goto out_nofid; - } - - if (!strcmp(".", name.data) || !strcmp("..", name.data)) { - err = -EISDIR; + err = check_name(name.data, pdu); + if (err < 0) { goto out_nofid; } @@ -3505,14 +3499,12 @@ static void coroutine_fn v9fs_renameat(void *opaque) goto out_err; } - if (name_is_illegal(old_name.data) || name_is_illegal(new_name.data)) { - err = -ENOENT; + err = check_name(old_name.data, pdu); + if (err < 0) { goto out_err; } - - if (!strcmp(".", old_name.data) || !strcmp("..", old_name.data) || - !strcmp(".", new_name.data) || !strcmp("..", new_name.data)) { - err = -EISDIR; + err = check_name(new_name.data, pdu); + if (err < 0) { goto out_err; } @@ -3617,12 +3609,8 @@ static void coroutine_fn v9fs_wstat(void *opaque) err = -EOPNOTSUPP; goto out; } - if (name_is_illegal(v9stat.name.data)) { - err = -ENOENT; - goto out; - } - if (!strcmp(".", v9stat.name.data) || !strcmp("..", v9stat.name.data)) { - err = -EISDIR; + err = check_name(v9stat.name.data, pdu); + if (err < 0) { goto out; } @@ -3755,13 +3743,8 @@ static void coroutine_fn v9fs_mknod(void *opaque) } trace_v9fs_mknod(pdu->tag, pdu->id, fid, mode, major, minor); - if (name_is_illegal(name.data)) { - err = -ENOENT; - goto out_nofid; - } - - if (!strcmp(".", name.data) || !strcmp("..", name.data)) { - err = -EEXIST; + err = check_name(name.data, pdu); + if (err < 0) { goto out_nofid; } @@ -3917,13 +3900,8 @@ static void coroutine_fn v9fs_mkdir(void *opaque) } trace_v9fs_mkdir(pdu->tag, pdu->id, fid, name.data, mode, gid); - if (name_is_illegal(name.data)) { - err = -ENOENT; - goto out_nofid; - } - - if (!strcmp(".", name.data) || !strcmp("..", name.data)) { - err = -EEXIST; + err = check_name(name.data, pdu); + if (err < 0) { goto out_nofid; } -- 2.47.3
