[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 01/13] Export hypervisor symbols



>>> On 25.09.13 at 16:03, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> wrote:
> On 09/25/2013 09:15 AM, Jan Beulich wrote:
>>>>> On 20.09.13 at 11:42, Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> wrote:
>>> +    case XENPF_get_symbols:
>>> +    {
>>> +        ret = xensyms_read(&op->u.symdata);
>>> +        if ( ret >= 0 && __copy_field_to_guest(u_xenpf_op, op, u.symdata) )
>>> +            ret = -EFAULT;
>>> +    }
>>> +    break;
>> This yields a positive return value if a symbol was found, 0 if none
>> was found, and negative on error. Can we avoid this non-standard
>> first aspect?
> 
> We need to know on the caller side when EOF is reached. There is no good
> error code that I can see that would be appropriate here. ERANGE or ENFILE
> are the closest I can imagine but EOF is not really an error so I am not 
> sure
> this would be the right thing.
> 
> We could look at first byte of the string and see where it's a 0 but 
> that's also
> somewhat non-standard. Encoding type or address as an invalid token also
> doesn't look nice.

Why don't you simply have the hypercall increment the passed in
symbol index. And if it didn't get incremented the called will know
there was no data returned.

>>> +
>>> +struct xenpf_symdata {
>>> +    /* IN variables */
>>> +    uint64_t xen_symnum;
>>> +
>>> +    /* OUT variables */
>>> +    uint64_t address;
>>> +    uint64_t type;
>> "type" and "xen_symnum" could easily be less than 64 bits wide.
> 
> I am trying to avoid 32-bit compatibility issues here. You will see 
> sometimes
> unnecessary uint64_t in other structures well for the same reason.

I'd encourage you to avoid that, and use padding fields instead.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.