This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
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.
Attachment:
newlib-nano-formatted-io-20140702.txt.tar.bz2
Description: BZip2 compressed data
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |