On Mon, 12 Mar 2018 11:00:39 -0500
Eric Blake <ebl...@redhat.com> wrote:
> On 03/12/2018 10:50 AM, nee wrote:
>
> >>> } else if (perm & P9_STAT_MODE_LINK) {
> >>> - int32_t ofid = atoi(extension.data);
> >>> - V9fsFidState *ofidp = get_fid(pdu, ofid);
> >>> + int ofid;
> >>
> >>
> >> 'unsigned int' and...
> >>
> >>> + V9fsFidState *ofidp;
> >>> +
> >>> + if (qemu_strtoi(extension.data, NULL, 10, &ofid)) {
> >>
> >>
> >> qemu_strtoui() might be smarter, per Greg's comments on v1.
> >>
> >>> + err = -EINVAL;
> >>> + goto out;
> >>> + }
> >>> + ofidp = get_fid(pdu, (int32_t)ofid);
> >>
> >>
> >> This cast is spurious.
> >>
> >> --
> >> Eric Blake, Principal Software Engineer
> >> Red Hat, Inc. +1-919-301-3266
> >> Virtualization: qemu.org | libvirt.org
> >
> > I did this because get_fid() takes an int32_t, not an unsigned int.
> > The struct V9fsFidState also uses an int32_t for its `fid` member. Do
> > you want me to change all these types, or just the function being used
> > here?
>
> I'll let Greg answer; he's more familiar with the 9p code (I was just
> commenting based on his initial answer to v1).
>
Yeah... as I was saying, fids are 32-bit unsigned per spec but the code is
using int32_t indeed. This is incorrect and it should be fixed.
