Re: xdrproc_t prototype

[Iain Hibbert <plunky%rya-online.net@localhost>]

On Thu, 25 Aug 2011, David Holland wrote:

> On Thu, Aug 25, 2011 at 07:23:02AM +0100, Iain Hibbert wrote:
>  > > > Ok I will investigate non-vararg prototype for xdrproc_t
>  > > >
>  > > > So far I only found one place where an xdrproc_t was called, which was
>  > > > using two arguments, so (XDR *, const char *) would work fine. I don't
>  > > > know what the point of providing a function with three args would be,
>  > > > unless it is possible to replace the handler that does the callback.
>  > >
>  > > welcome to sunrpc :(
>  > >
>  > > If you can figure out how to make it fit together without function
>  > > pointer casts, please do.
>  >
>  > I think the problem there would be that the prototypes of common functions
>  > are documented, so changing that is rewriting the whole API, a whole other
>  > deal.
>
> Well, right, the prototype has xdrproc_t, all these guys passing
> something else are just plain wrong :-)

yes, except they designed it that way :)

> (Since the amount of new code appearing using SunRPC is negligible,
> and the amount of it that's third party is small, and since it's all
> so nasty, I'd say that almost any arbitrary changes to make it cleaner
> are defensible.)
>
> I'm guessing that most of the functions that aren't (XDR *, const void *)
> are things like (XDR *, const struct foo *), right? Which can be fixed to
> be formally type-safe with only minor adjustments.

right, its supposed to be an opaque pointer (see xdr(3) for a list)

> I'm not going to demand that you do it, but it would be nice if it
> were easily possible to track down the places needing adjustment
> later. I suppose grepping the tree for (xdrproc_t) would be sufficient
> though.

yes, with my changes, most any user of this has to have a cast so they are
easily found. If the functions were changed from eg

bool_t
xdr_long(XDR *xdrs, long *lp)
{

        ...
}

to

bool_t
xdr_long(XDR *xdrs, void *arg)
{
        long *lp = arg;

        ...
}

would that count as an ABI or API change?  I guess technically it would
(for API) but functionally, there would be no change..

> I guess what I don't really like is putting casts inside the clnt_call
> macros...

its nasty, but is already full of casts..

iain