Re: [Fwd: Re: [PATCH] libibmad: Fixes for failures when not all ports of HCA are connected]

[brendan doyle <brendan.doyle-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>]

On 20/03/2013 22:24, Jason Gunthorpe wrote:
> On Wed, Mar 20, 2013 at 10:00:09PM +0000, brendan doyle wrote:
>
>>>>> As far as I can see the library is not documented at all, I can't find
>>>>> any man pages. So setting errno is not breaking the interface, and I
>>>>> would argue that if it adds value, which it does in this case, then
>>>>> what is the objection. Additionally I think if the approach is that
>>>>> errno is not set unless documented in a man page, then we should have
>>>>> some consistency, a quick grep of errno in libibmad reveals that errno
>>>>> is being set in other parts of the library, in libibumad too, we see
>>>>> errno set, but again it is not documented in the umad man pages.
>>>> Ideally we would have consistency amongst the IB libraries - try hard to
>>>> return -ERRNO like verbs, and only use errno for cases where an int return is
>>>> not possible.
>>> Since these calls return int I thought about this.  But I am
>>> worried about breaking users who may be explicitly checking for -1.
>>> OTOH nothing is documented.
>   
>> Well there are man pages for umad and verbs and though some verbs
>> man pages say errno is set for failure, they don't indicate what it
>> is set to, and umad man pages say nothing of errno. In any case the
>> proposed changes don't change the return value, and so won't break
>> existing users, again I can only see goodness being added, and fail
>> to see any negative implications?
> Patches adding inconsistency are not 'goodness'.

Where is the inconsistency? failure is still returned as -1, and errno 
is still
undefined, it is set in some parts of libibmad and not others, so nothing
has changed from a consumer perspective?

Also this is just one aspect of the proposed update the other hardens the
lib from trying to use an invalid SM LID.


>
> People have to use this stuff and try to write correct
> applications. Playing wack a mole on 'how does this call return
> errors' is a special kind of nightmare.

Again the patch does not break the existing API -1 is still failure.

>
> .. and look, I already got it wrong in my memory, verbs consistently
> uses the POSIX +ERRNO convention, not the kernel's -ERRNO convention.
>
> Ira, do you know any place that would break if +ERRNO is returned, or
> is this just a hypothetical? Even so, I'm not sure I have much
> sympathy for people checking against -1... that is only appropriate
> for FDs.
>
> Jason

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html