Re: [avr-libc-dev] Please have a look at avr-libc patch #3750

[Joerg Wunsch]

(Accumulated followup to several replies.)

As Dmitry K. wrote:

> It is possible to facilitate transition to a new variant:

>    1. To change the order of arguments 'put' as:
>       int (*put)(FILE *, char) --> int (*put)(char, FILE *)
>    It would allow to leave the asm program 'put' without changes.

But only these. ;-)

For C programs, they'd yield a compile-time error due to the different
function prototypes.

>    By the way, standard functions too have 'FILE *' the second argument.

Which I btw. find an illogical order.  But I agree, it might make some
sense in this case.

>    2. To clean the specification of arguments in 'stdio.h':
>       fdevopen(int (*put)(), int (*get)(), int opts __attribute__((unused)))
>    It will allow to compile existing programs without changes.

I disagree.  I don't want to poison our interface spec with
non-protoype function declarations.  After all, this is the year of
2005, and non-prototype function decl's have been deprecated in 1989.

> It would seem to me more useful instead of 'fdevopen' another way:
>    1. Standard files to have statically.

I'd rather allow for user-supplied buffers, so users don't have to
"buy" buffers for stdin, stdout, and stderr if they want only stdout.


As Bernard Fouché wrote:

> - I do not use fdevopen() because it calls malloc() and I don't need
> malloc/free/calloc anywhere else.

As the malloc()-less API change is long overdue, I'd rather move that
in by the same time.  We're going to have a new library minor version
number, so we are eligible for an API change anyway.

How about the following API:

#define __STDIO_FBUF_SIZE (sizeof(struct __file))
#define __STDIO_FPBUF_SIZE 40

FILE *fdevopen(int(*put)(char, FILE *), int(*get)(FILE *),
               void *fbuf, void *fpbuf);

Let's get rid of the old unused opts parameter; when I crafted the old
API, I did have something like Ted's proposal in mind, but didn't
think that to an end (and nobody seemed to be interested discussing
the approach by that time), that's why I reserved a third parameter
for possible later use.  Ted's approach has much more merit, so that
parameter can go away now.

Parameter fbuf, if passed as != NULL, will be used to store the struct
FILE * into.  __STDIO_FBUF_SIZE can be used to statically allocate
such buffer.

Parameter fpbuf, if passed as != NULL, will be used as the
floating-point formatting buffer (and scanf buffer for %[ format).
__STDIO_FPBUF_SIZE can be used to statically allocate that buffer.

If the latter parameters are passed as NULL, malloc() will be used
to allocate them (for fpbuf, it will only be allocated as needed),
thus remaining backwards-compatible.

As there's no good way to decide at link-time whether people will
really pass all parameters in a way no malloc() is used, the linker
will always drag a malloc() into the executable.  However, users will
be advised that they can safely use their own replacement malloc()
version, supposedly a null implementation, or a call to abort() or
such.  I hesitate to start yet another offspring version of vfscanf()
and vfprintf() to handle the malloc/non-malloc distinction.  That way,
a single RET instruction will be left from the malloc() function
referenced by the stdio functions, i.e. there will be two unused bytes
in the code.

Does that sound as a useful compromise to anybody?

> - I'd like a system where I reserve memory myself for a given stream and
> provide it to avr-libc.

See above.

> - I'd like to be able to use only stdout and not having to reserve
> memory for stdin/stderr if I do not need
> them. (or use only stdin and not stdout/err, etc)

That could be handled in a similar way.  Just use

  stdout = fdevopen(...);


As Mudiaga Obada wrote:

> This would become something like
> 
> 
> -             if (stream->put(c) != 0)
> +               if (stream->had__fdevopen2__opts) {
> +                 /* this stream was opened with fdeveopen2(a,b,
> __FDEVEOPEN2_INTERNAL_FLAG) */
> +                 uret = stream->put(stream, c)
> +               } else {
> +                 /* this stream was opened with fdeveopen2(a,b,0) */
> +                 uret = stream->put(c)
> +             }
> +               if (uret != 0)

I agree with Bernard here that I find the overhead of the if()
statement too heavy, just for compatibilitie's sake.  The stdio API is
now three years old, and people did have the chance to make their
experiences with it.  I think it's a good time for a well-thought API
change now, that is going to address all the concerns people did raise
in the past.  I'm even willing to take out that threat about the
instable API on top of

http://www.nongnu.org/avr-libc/user-manual/group__avr__stdio.html

afterwards. ;-)


As Bernard Fouché wrote:

> What about having arguments passed to put/get in such a way that an
> 'old' put would pop only 'c' and an uptodate one would pop 'c' &
> 'stream'?

They are not popped off the stack, but passed in registers anyway.
See above, I'm not completely advert to it, but it will only act as
failsafeness for assembly-written get and put handlers, while all C
implementations will break due to the changed function prototype.

Actually, it might be even a warning only. ;-)

-- 
cheers, J"org               .-.-.   --... ...--   -.. .  DL8DTL

http://www.sax.de/~joerg/                        NIC: JW11-RIPE
Never trust an operating system you don't have sources for. ;-)