This is the mail archive of the
newlib@sourceware.org
mailing list for the newlib project.
Re: Add XDR support (only built #if cygwin, for now)
On Feb 26 01:15, Charles Wilson wrote:
> Corinna Vinschen wrote:
> > Wouldn't it make sense to add a README to the xdr directory which
> > describes what to do to port XDR to another target? A few words
> > about where and how to add the callback function would be quite helpful.
>
> Sure.
>
> > Talking about Cygwin: I'm a bit puzzled why it's necessary to convert
> > the stderr printing into debug_printf messages at all. The error
> > reporting to stderr is simply part of the code. Even the glibc code
> > is still doing it unconditionally.
>
> But most embedded targets avoid glibc like the plague. In general it's
> a really bad idea for libraries to print error messages -- doing so
> could interfere with the actual application's HMI. OTOH, if there is
> any library that could be said to "own" the standard IO streams, and
> thus be allowed to inject its own error messages into them, it's the C
> runtime library...
It seems I expressed myself terribly bad. What I was trying to say
is
- The idea to redirect all error messages to the wrapper call in
xdr_private.c is a good idea in general. I don't think you should
have mixed feelings about it.
- *Only* for Cygwin I would prefer to keep the default behaviour
to print to stderr, since that's what glibc is doing as well.
The redirection to debug_printf is not necessary.
> Hmmm...I begin to believe, for better harmony with embedded targets,
> it'd be better to default to /nothing/ (except for setting errno) unless
> the client has specifically registered an appropriate callback for those
> error messages.
I agree.
> Now, if you like, perhaps cygwin should register a callback that simply
> prints to stderr (rather than debug strace), so that for cygwin the XDR
> functions act "just like" glibc's...
Yes, that sounds good.
> For those frequent occasions where we run out of memory. (It's actually
> pointless to specifically direct XDR's out-of-memory messages to the
> debug strace, because I'm pretty sure cygwin's malloc would already have
> indicated the out-of-memory on that debug strace anyway...)
Yep.
> Alternatively, I could leave in the fprintf(), but simply guard it with
> #if HAVE_STDIO (or whatever). That'd be more in line with the glibc
> implementation, with only the absolute minimal necessary accommodation
> to resource-limited embedded targets. But...I don't like it. <g>
Indeed.
> > As for double, I'm not at all fluent with libm stuff, but it looks like
> > tweaking the code for newlib is not overly complicated.
> > If _DOUBLE_IS_32BITS is set, call xdr_float from xdr_double.
> > Replace `#ifdef IEEEFP' with `#if defined (__IEEE_LITTLE_ENDIAN) ||
> > defined (__IEEE_BIG_ENDIAN)'.
>
> OK.
>
> > In xdr_double, replace BYTE_ORDER == BIG_ENDIAN with __IEEE_BIG_ENDIAN.
>
> Do you mean
> -#if BYTE_ORDER == BIG_ENDIAN
> +#ifdef __IEEE_BIG_ENDIAN
> ?
Yes, that was the idea. This and __IEEE_LITTLE_ENDIAN are used throughout
newlib/libm code so it makes sense to reuse them here.
> > Last but not least, if none of __IEEE_LITTLE_ENDIAN and __IEEE_BIG_ENDIAN
> > is defined, disable float support entirely *for now*. The code in
> > xdr_float.c only handles IEEEFP or VAX floats. Other formats require
> > some additional target-specific code.
>
> OK.
>
> > Would you mind to do that?
>
> Not a problem. It may take a while, and I won't actually be able to
> test it, except artificially. I'll present all of these requested
> changes as an interdiff, for easier review.
Just a single patch with everything would be ok, too, in this case.
> >> add some intelligence to optionally exclude those limited portions of
> >> the XDR code that require stdio to work...
> >
> > I think that's sufficiently covered.
> >
> > Well, stdio is used only in xdr_private.c, so, see above.
>
> And maybe not even there, if the by-default-only-errno route is chosen.
> However, xdrstdio.[h|c] definitely uses stdio, as do the
> xdrstdio_create() and related functions declared in xdr.h (FILE*
> arguments). So those need to be guarded.
The stdio stuff is only pulled in if the application actually uses the
functions from xdrstdio.c, and if the application uses them, it needs
stdio. So I don't see how that hurts.
> > As for malloc,
> > that seems to be nicely wrapped by mem_alloc and mem_free macros, so
> > targets can override them as needed (README?).
>
> Ack.
>
> Should that part of rpc/types.h be updated?
>
> -#define mem_alloc(bsize) calloc(1, bsize)
> -#define mem_free(ptr, bsize) free(ptr)
> +#ifndef mem_alloc
> +#define mem_alloc(bsize) calloc(1, bsize)
> +#endif
> +#ifndef mem_free
> +#define mem_free(ptr, bsize) free(ptr)
> +#endif
Good idea.
> > However, I see that malloc and free are used in xdr_sizeof.c, rather than
> > mem_alloc and mem_free. Shouldn't that be fixed?
>
> That's from the inherited Sun RPC code, but I agree it should be fixed.
>
> > With the above changes I don't actually see a reason to restrict the
> > XDR build to Cygwin.
>
> Weeeellll, maybe not -- but it ain't gonna be ME that activates a giant
> chunk of new code for 173 different platforms without testing them!
Ok.
Corinna
--
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat