From: Christian Schoenebeck <[email protected]> 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 Link: https://lore.kernel.org/qemu-devel/0573103880129eb543f07b68c77e86f2f572f6bf.1780072238.git.qemu_...@crudebyte.com Signed-off-by: Christian Schoenebeck <[email protected]> (cherry picked from commit 116db2986b11c914217bbd1547815b6c7efb944a) Signed-off-by: Michael Tokarev <[email protected]> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index 34ac2c3c9a..9718daab61 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -1802,6 +1802,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; @@ -2152,13 +2171,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; } @@ -2840,13 +2854,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; } @@ -3034,13 +3043,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; } @@ -3127,13 +3131,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; } @@ -3364,13 +3363,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
