Re: [Xen-devel] [PATCH 1/2] xen: fix usage of ENODATA

[Roger Pau Monnà <roger.pau@xxxxxxxxxx>]

On 15/04/14 14:33, Egger, Christoph wrote:
> On 15.04.14 10:33, Roger Pau Monne wrote:
>> ENODATA doesn't exist on FreeBSD, so ENODATA errors returned by the
>> hypervisor are translated to ENOENT.
>>
>> Also, the error code is returned in errno if the call returns -1, so
>> compare the error code with the value in errno instead of the value
>> returned by the function.
>>
>> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx
>> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
>> Cc: Anthony Perard <anthony.perard@xxxxxxxxxx>
>> ---
>>  xen-all.c |    8 ++++++--
>>  1 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen-all.c b/xen-all.c
>> index 0b4d934..4026b7b 100644
>> --- a/xen-all.c
>> +++ b/xen-all.c
>> @@ -499,11 +499,15 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
>>                                   start_addr >> TARGET_PAGE_BITS, npages,
>>                                   bitmap);
>>      if (rc < 0) {
>> -        if (rc != -ENODATA) {
>> +#ifdef ENODATA
>> +        if (errno == ENODATA) {
>> +#else
>> +        if (errno == ENOENT) {
>> +#endif
> 
> This does not look good to me.
> I suggest this:
> 
> #ifndef ENODATA
> #define ENODATA  ENOENT
> #endif

Hello Christoph,

Thanks for the review, I slightly prefer my original proposal over what
you suggest, I think it's more clear that there are different error
codes, while your suggestion simply masks the fact than FreeBSD doesn't
have ENODATA. Anyway, I wouldn't mind changing it if the maintainer
prefers your approach.

Roger.


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