| Message ID | 149399502436.29022.4452572475773635974.stgit@bahia.lan |
|---|---|
| State | New |
| Headers | show |
On 05/05/2017 09:37 AM, Greg Kurz wrote: > These v9fs_co_name_to_path() call sites have always been around. I guess > no care was taken to check the return value because the name_to_path > operation could never fail at the time. This is no longer true: the > handle and synth backends can already fail this operation, and so will the > local backend soon. > > Signed-off-by: Greg Kurz <groug@kaod.org> > --- > hw/9pfs/9p.c | 36 +++++++++++++++++++++++++----------- > 1 file changed, 25 insertions(+), 11 deletions(-) > > @@ -2588,8 +2591,11 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp, > new_name = g_malloc0(end - old_name + name->size + 1); > strncat(new_name, old_name, end - old_name); > strncat(new_name + (end - old_name), name->data, name->size); Ad long as you're here, you could replace this strncat mess with the shorter: new_name = g_strdup_printf("%.*s%.*s", end - old_name, old_name, name->size, name->data); (or with further simplifications if you have NUL-terminated data). But that can be a separate cleanup. Reviewed-by: Eric Blake <eblake@redhat.com>
On Fri, 5 May 2017 11:55:43 -0500 Eric Blake <eblake@redhat.com> wrote: > On 05/05/2017 09:37 AM, Greg Kurz wrote: > > These v9fs_co_name_to_path() call sites have always been around. I guess > > no care was taken to check the return value because the name_to_path > > operation could never fail at the time. This is no longer true: the > > handle and synth backends can already fail this operation, and so will the > > local backend soon. > > > > Signed-off-by: Greg Kurz <groug@kaod.org> > > --- > > hw/9pfs/9p.c | 36 +++++++++++++++++++++++++----------- > > 1 file changed, 25 insertions(+), 11 deletions(-) > > > > > @@ -2588,8 +2591,11 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp, > > new_name = g_malloc0(end - old_name + name->size + 1); > > strncat(new_name, old_name, end - old_name); > > strncat(new_name + (end - old_name), name->data, name->size); > > Ad long as you're here, you could replace this strncat mess with the > shorter: > > new_name = g_strdup_printf("%.*s%.*s", end - old_name, old_name, > name->size, name->data); > > (or with further simplifications if you have NUL-terminated data). But > that can be a separate cleanup. > Yes you're right, this definitely looks better than the strncat() logic. I guess I'll do this in a separate patch. Thanks for the suggestion. > Reviewed-by: Eric Blake <eblake@redhat.com> >
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c index c80ba67389ce..dc5250b342fa 100644 --- a/hw/9pfs/9p.c +++ b/hw/9pfs/9p.c @@ -2576,7 +2576,10 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp, err = -EINVAL; goto out; } - v9fs_co_name_to_path(pdu, &dirfidp->path, name->data, &new_path); + err = v9fs_co_name_to_path(pdu, &dirfidp->path, name->data, &new_path); + if (err < 0) { + goto out; + } } else { old_name = fidp->path.data; end = strrchr(old_name, '/'); @@ -2588,8 +2591,11 @@ static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp, new_name = g_malloc0(end - old_name + name->size + 1); strncat(new_name, old_name, end - old_name); strncat(new_name + (end - old_name), name->data, name->size); - v9fs_co_name_to_path(pdu, NULL, new_name, &new_path); + err = v9fs_co_name_to_path(pdu, NULL, new_name, &new_path); g_free(new_name); + if (err < 0) { + goto out; + } } err = v9fs_co_rename(pdu, &fidp->path, &new_path); if (err < 0) { @@ -2669,20 +2675,26 @@ out_nofid: v9fs_string_free(&name); } -static void coroutine_fn v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir, - V9fsString *old_name, - V9fsPath *newdir, - V9fsString *new_name) +static int coroutine_fn v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir, + V9fsString *old_name, + V9fsPath *newdir, + V9fsString *new_name) { V9fsFidState *tfidp; V9fsPath oldpath, newpath; V9fsState *s = pdu->s; - + int err; v9fs_path_init(&oldpath); v9fs_path_init(&newpath); - v9fs_co_name_to_path(pdu, olddir, old_name->data, &oldpath); - v9fs_co_name_to_path(pdu, newdir, new_name->data, &newpath); + err = v9fs_co_name_to_path(pdu, olddir, old_name->data, &oldpath); + if (err < 0) { + goto out; + } + err = v9fs_co_name_to_path(pdu, newdir, new_name->data, &newpath); + if (err < 0) { + goto out; + } /* * Fixup fid's pointing to the old name to @@ -2694,8 +2706,10 @@ static void coroutine_fn v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir, v9fs_fix_path(&tfidp->path, &newpath, strlen(oldpath.data)); } } +out: v9fs_path_free(&oldpath); v9fs_path_free(&newpath); + return err; } static int coroutine_fn v9fs_complete_renameat(V9fsPDU *pdu, int32_t olddirfid, @@ -2729,8 +2743,8 @@ static int coroutine_fn v9fs_complete_renameat(V9fsPDU *pdu, int32_t olddirfid, } if (s->ctx.export_flags & V9FS_PATHNAME_FSCONTEXT) { /* Only for path based fid we need to do the below fixup */ - v9fs_fix_fid_paths(pdu, &olddirfidp->path, old_name, - &newdirfidp->path, new_name); + err = v9fs_fix_fid_paths(pdu, &olddirfidp->path, old_name, + &newdirfidp->path, new_name); } out: if (olddirfidp) {
These v9fs_co_name_to_path() call sites have always been around. I guess no care was taken to check the return value because the name_to_path operation could never fail at the time. This is no longer true: the handle and synth backends can already fail this operation, and so will the local backend soon. Signed-off-by: Greg Kurz <groug@kaod.org> --- hw/9pfs/9p.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-)