This is the mail archive of the
newlib@sourceware.org
mailing list for the newlib project.
Re: [PATCH]Introducing small foot-print implementation for non-wide-char formatted IO
- From: Jeff Johnston <jjohnstn at redhat dot com>
- To: "Bin.Cheng" <amker dot cheng at gmail dot com>
- Cc: Bin Cheng <bin dot cheng at arm dot com>, newlib at sourceware dot org
- Date: Fri, 4 Jul 2014 13:22:29 -0400 (EDT)
- Subject: Re: [PATCH]Introducing small foot-print implementation for non-wide-char formatted IO
- Authentication-results: sourceware.org; auth=none
- References: <000801cf8c4a$aea744e0$0bf5cea0$ at arm dot com> <509181399 dot 32662868 dot 1403739041645 dot JavaMail dot zimbra at redhat dot com> <001001cf9111$5f25fb70$1d71f250$ at arm dot com> <33876190 dot 2470303 dot 1404326677955 dot JavaMail dot zimbra at redhat dot com> <CAHFci2-VYZbyxn8XU+5_KBWuFn_qWepj29OLQLeEb9gu42J+0A at mail dot gmail dot com> <1513142129 dot 2590176 dot 1404337817709 dot JavaMail dot zimbra at redhat dot com> <CAHFci28whRMfiTm=k7Jr59GAo4TOmU8ucBwHtHqGFT6jg5fBDA at mail dot gmail dot com>
Patch applied.
-- Jeff J.
----- Original Message -----
From: "Bin.Cheng" <amker.cheng@gmail.com>
To: "Jeff Johnston" <jjohnstn@redhat.com>
Cc: "Bin Cheng" <bin.cheng@arm.com>, newlib@sourceware.org
Sent: Wednesday, July 2, 2014 6:10:44 PM
Subject: Re: [PATCH]Introducing small foot-print implementation for non-wide-char formatted IO
Hi Jeff,
The updated patch is attached. Please review.
Thanks,
bin
On Wed, Jul 2, 2014 at 10:50 PM, Jeff Johnston <jjohnstn@redhat.com> wrote:
> Please delete the 3rd clause as in vfwprintf.c. You can also renumber clause 4 to 3.
>
> -- Jeff J.
>
> ----- Original Message -----
> From: "Bin.Cheng" <amker.cheng@gmail.com>
> To: "Jeff Johnston" <jjohnstn@redhat.com>
> Cc: "Bin Cheng" <bin.cheng@arm.com>, newlib@sourceware.org
> Sent: Wednesday, July 2, 2014 5:31:56 PM
> Subject: Re: [PATCH]Introducing small foot-print implementation for non-wide-char formatted IO
>
> On Wed, Jul 2, 2014 at 7:44 PM, Jeff Johnston <jjohnstn@redhat.com> wrote:
>> Hi Bin,
>>
>> Sorry for the long delay in replying. Here in Canada, we had a holiday on Tuesday so I have been on vacation since Friday.
>>
>> A few comments on the latest patch:
>>
>> 1. You have files that have the Berkeley advertising clause in them. You need to remove those clauses.
>> See: ftp://ftp.cs.berkeley.edu/pub/4bsd/README.Impt.License.Change
>
> Hi Jeff, thanks for correcting the mistake.
> There are different kinds of modification in files from
> newlib/stdio/libc. Files like vfwprintf.c just delete the 3rd clause,
> while files like vfwscanf.c make bigger changes, so which kind of
> statement should I follow?
>
> Thanks,
> bin
>
>>
>> 2. You have a typo in the README regarding long double. I took the liberty of rewording a bit as well:
>>
>> The only exception is the configuration option provides limited support for
>> long double. Internally, the nano formatted I/O functions use double so accuracy is only guaranteed
>> to double precision.
>>
>> 3. Regarding the discussion on the newlib mailing list about the standard supported, I have no issue with you just stating that you only support the C89 Standard.
>>
>> -- Jeff J.
>>
>>
>> ----- Original Message -----
>> From: "Bin Cheng" <bin.cheng@arm.com>
>> To: "Jeff Johnston" <jjohnstn@redhat.com>
>> Cc: newlib@sourceware.org
>> Sent: Thursday, June 26, 2014 3:36:37 AM
>> Subject: RE: [PATCH]Introducing small foot-print implementation for non-wide-char formatted IO
>>
>> Hi Jeff,
>> Thanks very much for rephrasing the wording. Here is the revised patch incorporating your comments, please review.
>>
>> Thanks,
>> bin
>>
>> -----Original Message-----
>> From: newlib-owner@sourceware.org [mailto:newlib-owner@sourceware.org] On Behalf Of Jeff Johnston
>> Sent: Thursday, June 26, 2014 7:31 AM
>> To: Bin Cheng
>> Cc: newlib@sourceware.org
>> Subject: Re: [PATCH]Introducing small foot-print implementation for non-wide-char formatted IO
>>
>> Hi Bin,
>>
>> Looks interesting. I have a few minor comments/questions:
>>
>> 1. Your README section could be clarified. Perhaps something like this:
>>
>> +`--enable-newlib-nano-formatted-io'
>> + This builds NEWLIB with a special implementation of formatted I/O functions, designed to
>> + lower the size of applications on small systems with size constraint issues. This option does not affect
>> + wide-char formatted I/O functions. Some notes about the feature:
>> +
>> + 1) The non-wide-char formatted I/O functions only support the C89
>> + standard
>> +
>> + 2) Floating-point support is split out of the formatted I/O code into weak functions which are not linked
>> + by default. Programs that need floating-point I/O support must explicitly
>> + request linking of one or both of the floating-point functions: _printf_float or _scanf_float. This can be done
>> + at link time using the -u option which can be passed to either gcc or ld. The -u option forces the link to
>> + resolve those function references.
>> +
>> + 3) Integer-only versions of the formatted I/O functions (the iprint/iscanf family) simply alias their regular counter-parts.
>> + The affected functions are:
>> +
>> + diprintf vdiprintf
>> +
>> + siprintf fiprintf iprintf sniprintf asiprintf asniprintf
>> +
>> + siscanf fiscanf iscanf
>> +
>> + viprintf vfiprintf vsiprintf vsniprintf vasiprintf vasniprintf
>> +
>> + viscanf vfiscanf vsiscanf
>> +
>> + _diprintf_r _vdiprintf_r
>> +
>> + _siprintf_r _fiprintf_r _iprintf_r _sniprintf_r _asiprintf_r
>> + _asniprintf_r
>> +
>> + _siscanf_r _fiscanf_r _iscanf_r
>> +
>> + _viprintf_r _vfiprintf_r _vsiprintf_r _asniprintf_r _vasiprintf_r
>> + _vasniprintf_r
>> +
>> + _viscanf_r _vfiscanf_r _vsiscanf_r
>> +
>> +
>> + 4) As mentioned, the option does not affect wide-char formatted I/O. The following configuration options are ignored for non-wide-char
>> + formatted I/O functions:
>> +
>> + enable-newlib-io-pos-args
>> + enable-newlib-io-c99-formats
>> + enable-newlib-io-long-long
>> + enable-newlib-io-long-double
>> +
>>
>> 2. In your original README you mention:
>>
>> + Additionally, "enable-newlib-io-float" is no longer needed because of
>> + changes to the handling of floating-point formatted IO.
>>
>> but you have checks for the FLOATING_POINT flag in your code. I did not address this comment in the README edit above.
>>
>> The FLOATING_POINT flag is tied to the NO_FLOATING_POINT flag being undefined. This flag is tied to the --enable-newlib-io-float option (see configure.host). So, if the end-user performs a --disable-newlib-io-float, it removes a bit of code, some of which does nothing meaningful unless the end-user has explicitly asked for floating-point support.
>>
>> Would it not be clearer to state something like: "Floating-point format specifiers are, by default, recognized, but if the floating-point functions are not linked in, this may result in undefined behavior. If the application does not support floating-point, one can use the --disable-newlib-io-float configuration option and floating-point specifiers will not be recognized. Use of this option further reduces code size."
>>
>> 3. There should be some note about long double support. It appears that you partially support long double values by converting them to double precision internally.
>>
>> 4. For attribute flags, please use _ATTRIBUTE(__weak__) and _ATTRIBUTE(__alias__) though this isn't done everywhere, it would be nice to start to be consistent.
>>
>> -- Jeff J.
>>
>> ----- Original Message -----
>> From: "Bin Cheng" <bin.cheng@arm.com>
>> To: newlib@sourceware.org
>> Sent: Friday, June 20, 2014 1:44:17 AM
>> Subject: [PATCH]Introducing small foot-print implementation for non-wide-char formatted IO
>>
>> Hi,
>>
>> This is the second try to merge our work of formatted IO for small systems originated from Newlib-nano library, in project "GNU Tools for ARM Embedded Processors". For the first time try, please refer to archived message at:
>> https://sourceware.org/ml/newlib/2013/msg00435.html
>>
>> Since Newlib-nano in our project has been adopted/verified by many ARM embedded developers and is relatively stable as far as I can see, here I am trying again to upstream it. In this patch I rebased/refactored code against the latest trunk. I also introduced a new configuration option "--enable-newlib-nano-formatted-io" to control this small foot-print implementation of formatted IO. The new option can be used just like other code size saving ones, and there is some straightforward description I added for it in newlib/README, which is quoted here:
>>
>> +`--enable-newlib-nano-formatted-io'
>> + NEWLIB has a special implementation for formatted IO functions, it has
>> + below features:
>> + 1) It only supports C89 standard and would never include other
>> features.
>> + 2) It removes direct dependency on floating-point IO handling code.
>> + Programs that need to handle floating-point IO must now explicitly
>> + request the feature by providing options "-u _printf_float" or
>> + "-u _scanf_float" at linking command line.
>> + 3) It removes now redundant integer-only implementation of the
>> + printf/scanf family of routines (iprintf/iscanf, etc.). These
>> + functions now alias the standard routines. This avoids the risk
>> + of getting duplicated functionality when these operations are
>> + needed. The affected functions are:
>> +
>> + diprintf vdiprintf
>> +
>> + siprintf fiprintf iprintf sniprintf asiprintf asniprintf
>> +
>> + siscanf fiscanf iscanf
>> +
>> + viprintf vfiprintf vsiprintf vsniprintf vasiprintf vasniprintf
>> +
>> + viscanf vfiscanf vsiscanf
>> +
>> + _diprintf_r _vdiprintf_r
>> +
>> + _siprintf_r _fiprintf_r _iprintf_r _sniprintf_r _asiprintf_r
>> + _asniprintf_r
>> +
>> + _siscanf_r _fiscanf_r _iscanf_r
>> +
>> + _viprintf_r _vfiprintf_r _vsiprintf_r _asniprintf_r _vasiprintf_r
>> + _vasniprintf_r
>> +
>> + _viscanf_r _vfiscanf_r _vsiscanf_r
>> +
>> +
>> + With this implementation, the following newlib configuration options
>> + are no longer available for non-wide-char formatted IO:
>> +
>> + enable-newlib-io-pos-args
>> + enable-newlib-io-c99-formats
>> + enable-newlib-io-long-long
>> + enable-newlib-io-long-double
>> +
>> + Additionally, "enable-newlib-io-float" is no longer needed because of
>> + changes to the handling of floating-point formatted IO.
>> +
>> + This option enables the nano-formatted-IO implementation, and should
>> + be used for small systems with very limited memory.
>> + Disabled by default.
>> +
>>
>> I know there are concerns about maintenance burden of keeping two implementations of formatted IO, which I hope can be addressed by below
>> comments:
>> 1) The two implementations are totally independent to each other, in other words, there is no shared code which need to be dealt with in future patches.
>> 2) The nano version of implementation is designed for small systems with very limited resources only, so it is restricted to C89 standard. I think it's reasonable to setup a rule after merging that we will never include features outside of C89.
>> 3) The nano version of implementation is quite modularized, with most codes
>> added by several new files. Other than that, it's just like other code
>> size saving options.
>> 4) The nano version of implementation has been widely used since it was first introduced in 2012. Because it's very simple, there is not many issues reported so far, and I don't expect there will be many issues after merging.
>>
>> With these considerations, I guess it's not inappropriate for it to be included in Newlib. So any comments?
>>
>> BTW, I compressed the patch since it exceeds size limitation of newlib mailing list.
>>
>> Thanks,
>> bin
>>
>> 2014-06-20 Bin Cheng <bin.cheng@arm.com>
>>
>> * README (--enable-newlib-nano-formatted-io): Describe.
>> * acconfig.h (_NANO_FORMATTED_IO): Undef.
>> * newlib.hin (_NANO_FORMATTED_IO): Undef.
>> * configure.in (--enable-newlib-nano-formatted-io): New option.
>> * configure: Regenerated.
>> * libc/configure.in (--enable-newlib-nano-formatted-io): New option.
>> * libc/configure: Regenerated.
>> * libc/stdio/Makefile.am (NEWLIB_NANO_FORMATTED_IO): Support new
>> configuration option.
>> * libc/stdio/Makefile.in: Regenerated.
>> * libc/stdio/asnprintf.c (_asniprintf_r, asniprintf): Use
>> _NANO_FORMATTED_IO to declare alias prototypes.
>> * libc/stdio/asprintf.c (_asiprintf_r, asiprintf): Ditto.
>> * libc/stdio/dprintf.c (_diprintf_r, diprintf): Ditto.
>> * libc/stdio/fprintf.c (_fiprintf_r, fiprintf): Ditto.
>> * libc/stdio/fscanf.c (fiscanf, _fiscanf_r): Ditto.
>> * libc/stdio/printf.c (_iprintf_r, iprintf): Ditto.
>> * libc/stdio/scanf.c (iscanf, _iscanf_r): Ditto.
>> * libc/stdio/snprintf.c (_sniprintf_r, sniprintf): Ditto.
>> * libc/stdio/sprintf.c (_siprintf_r, siprintf): Ditto.
>> * libc/stdio/sscanf.c (siscanf, _siscanf_r): Ditto.
>> * libc/stdio/vasnprintf.c (_vasniprintf_r, vasniprintf): Ditto.
>> * libc/stdio/vasprintf.c (vasiprintf, _vasiprintf_r): Ditto.
>> * libc/stdio/vdprintf.c (_vdiprintf_r, vdiprintf): Ditto.
>> * libc/stdio/vprintf.c (viprintf, _viprintf_r): Ditto.
>> * libc/stdio/vscanf.c (viscanf, _viscanf_r): Ditto.
>> * libc/stdio/vsnprintf.c (vsniprintf, _vsniprintf_r): Ditto.
>> * libc/stdio/vsprintf.c (vsiprintf, _vsiprintf_r): Ditto.
>> * libc/stdio/vsscanf.c (vsiscanf, _vsiscanf_r): Ditto.
>> * libc/stdio/nano-vfprintf.c: New file.
>> * libc/stdio/nano-vfprintf_float.c: New file.
>> * libc/stdio/nano-vfprintf_i.c: New file.
>> * libc/stdio/nano-vfprintf_local.h: New file.
>> * libc/stdio/nano-vfscanf.c: New file.
>> * libc/stdio/nano-vfscanf_float.c: New file.
>> * libc/stdio/nano-vfscanf_i.c: New file.
>> * libc/stdio/nano-vfscanf_local.h: New file.