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

[Mudiaga Obada]

>>For example if the new fdevopen were named fdevopen2 (or what ever),
>>and existing fdevopen is changed to a simple wrapper call to
>>fdevopen2 using __opts to tell it to use the new style
>>callback. "struct __file" could then use a union for the get & put
>>pairs, and test __opts before calling the correct callback.
> 
> 
> Unfortunately, that's the least of the problems with the API change.
> The more important thing is that the internal `get' and `put'
> functions change their API, as they get passed the FILE * down --
> and this is basically unavoidable, as that's the actual purpose of
> the suggested patch.

Unless they get passed further down than i can see from the patch,  I
don't yet see why the changes can't be made without breaking fdevopen()
as the internal get() and put() callbacks are ... internal.

fdevopen and uart_putchar seem to be the only external APIs the patch
changes. (I don't mind if uart_putchar is changed).

Here is what i was thinking of in code:

Take for these lines of patch:

-               if (stream->put(c) != 0)
+               if (stream->put(stream, c) != 0)


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)


With the new fdevopen named fdevopen2, and for compatibility fdevopen
implemented as

        return fdeveopen2((cast)put, (cast)get, __FDEVEOPEN2_INTERNAL_FLAG)






-------- Original Message --------
Subject: Re: [avr-libc-dev] Please have a look at avr-libc patch #3750
Date: Sun, 04 Sep 2005 23:10:40 +0200
From: Mudiaga Obada <address@hidden>
To: Joerg Wunsch <address@hidden>
References: <address@hidden>

I looked over the patch and it is something that makes sense.

However I don't like the idea of making this API change that would break
existing code - especially since the functionality could be added
without breaking existing code.

For example if the new fdevopen were named fdevopen2 (or what ever), and
existing fdevopen is changed to a simple wrapper call to fdevopen2 using
__opts to tell it to use the new style callback. "struct __file" could
then use a union for the get & put pairs, and test __opts before calling
the correct callback.

I would rather have this "extra cost" of checking __opts than have
frustrated users fustrating you developers. So I would not even
deprecate fdevopen. If this "extra cost" is too expensive for someone
for whatever reasons, "they" could add a compile time option (configure
--without-fdevopen || configure --without-fdevopen2 ) that would remove
support for one or the other.

Regards,

Mudiaga Obada


Joerg Wunsch wrote:

> Patch URL:
>
> https://savannah.nongnu.org/patch/?func=detailitem&item_id=3750
>
>  ...
>
> My only concern about it is that it would constitute an API change for
> the stdio API, so all users of the current API would have to change
> their sources.