0002714: sprintf should not pull in most of stdio.h code

(0006274)
vda
04-01-08 11:13

Since I have a good test case, I can provide more info (link map rules!). seeks are pulled in because of __UCLIBC_HAS_STDIO_AUTO_RW_TRANSITION__ here:

#ifdef __UCLIBC_HAS_WCHAR__
int attribute_hidden __stdio_trans2w_o(FILE * __restrict stream, int oflag)
#else
int attribute_hidden __stdio_trans2w(FILE * __restrict stream)
#endif
{
...
        if (__STDIO_STREAM_IS_READING(stream)) {
                if (!__FEOF_UNLOCKED(stream)) {
#ifdef __UCLIBC_HAS_STDIO_AUTO_RW_TRANSITION__
                        /* Need to seek to correct position if we have buffered
                         * read data or ungots. If appending, we might as well
                         * seek to the end.
                         *
                         * NOTE: If the OS does not handle append files correctly,
                         * this is insufficient since we would need to seek to
                         * the end even if not reading.*/
                        if (((__STDIO_STREAM_BUFFER_RAVAIL(stream))
                                 || (stream->__modeflags & __FLAG_UNGOT))
                                && fseek(stream, 0L,
...

which is pulled in because of STDIO_STREAM_TRANS_TO_WRITE here:

int VFPRINTF (FILE * __restrict stream,
                          register const FMT_TYPE * __restrict format,
                          va_list arg)
{
...
        if
#ifdef L_vfprintf
        (!__STDIO_STREAM_IS_NARROW_WRITING(stream)
         && __STDIO_STREAM_TRANS_TO_WRITE(stream, __FLAG_NARROW))
#else
        (!__STDIO_STREAM_IS_WIDE_WRITING(stream)
         && __STDIO_STREAM_TRANS_TO_WRITE(stream, __FLAG_WIDE))
#endif

(0006284)
vda
04-01-08 12:04

Seems like it can be improved in _vfprintf.c / old_vfprintf.c
by factoring out "the core" of vprintf() into separate function
(say, "vprintf_internal_unlocked()"), so that:

* vprintf() does locking and __STDIO_STREAM_TRANS_TO_WRITE thing,
  then calls vprintf_internal_unlocked();
* vsnprintf, vdprintf.c, vasprintf.c all use
  vprintf_internal_unlocked() directly
  [and do not need to set up fake locking now]

(0006294)
vda
04-01-08 14:50

2.patch implements this solution.
3.patch removes unneeded initialization of locking.

If you are going to apply it, take a look at vsnprintf.c's comment about __bufputc - it seems to be bogus, AFAICS __bufputc initialization has nothing to do with __user_locking. (I did grep around).

Didn't look yet whether it works with old_vfprintf.c.

(0006304)
vda
04-01-08 15:22

These patches save Note: 0000400 bytes compared to unpatched uclibc.

Next target can be removing __stdio_fwrite + __stdio_WRITE, as they should not be needed for sprintf too - projected to save Note: 0000300 bytes.

(0006454)
vda
04-08-08 17:54

v2 of patches should be more correct wrt old_vsprintf (it is just left untouched now)

(0006494)
vda
04-09-08 12:52

Applied as revisions 21683 and 21684.

Issue History
Date Modified	Username	Field	Change
04-01-08 09:26	vda	New Issue
04-01-08 09:26	vda	Status	new => assigned
04-01-08 09:26	vda	Assigned To	=> uClibc
04-01-08 11:13	vda	Note Added: 0006274
04-01-08 12:04	vda	Note Added: 0006284
04-01-08 14:46	vda	File Added: 2.patch
04-01-08 14:46	vda	File Added: 3.patch
04-01-08 14:50	vda	Note Added: 0006294
04-01-08 15:22	vda	Note Added: 0006304
04-08-08 17:53	vda	File Added: v2_2.patch
04-08-08 17:53	vda	File Added: v2_3.patch
04-08-08 17:54	vda	Note Added: 0006454
04-09-08 12:52	vda	Status	assigned => closed
04-09-08 12:52	vda	Note Added: 0006494
04-09-08 12:52	vda	Resolution	open => fixed

Notes
(0006274) vda 04-01-08 11:13	Since I have a good test case, I can provide more info (link map rules!). seeks are pulled in because of __UCLIBC_HAS_STDIO_AUTO_RW_TRANSITION__ here: #ifdef __UCLIBC_HAS_WCHAR__ int attribute_hidden __stdio_trans2w_o(FILE * __restrict stream, int oflag) #else int attribute_hidden __stdio_trans2w(FILE * __restrict stream) #endif { ... if (__STDIO_STREAM_IS_READING(stream)) { if (!__FEOF_UNLOCKED(stream)) { #ifdef __UCLIBC_HAS_STDIO_AUTO_RW_TRANSITION__ /* Need to seek to correct position if we have buffered * read data or ungots. If appending, we might as well * seek to the end. * * NOTE: If the OS does not handle append files correctly, * this is insufficient since we would need to seek to * the end even if not reading./ if (((__STDIO_STREAM_BUFFER_RAVAIL(stream)) \|\| (stream->__modeflags & __FLAG_UNGOT)) && fseek(stream, 0L, ... which is pulled in because of STDIO_STREAM_TRANS_TO_WRITE here: int VFPRINTF (FILE __restrict stream, register const FMT_TYPE * __restrict format, va_list arg) { ... if #ifdef L_vfprintf (!__STDIO_STREAM_IS_NARROW_WRITING(stream) && __STDIO_STREAM_TRANS_TO_WRITE(stream, __FLAG_NARROW)) #else (!__STDIO_STREAM_IS_WIDE_WRITING(stream) && __STDIO_STREAM_TRANS_TO_WRITE(stream, __FLAG_WIDE)) #endif

(0006284) vda 04-01-08 12:04	Seems like it can be improved in _vfprintf.c / old_vfprintf.c by factoring out "the core" of vprintf() into separate function (say, "vprintf_internal_unlocked()"), so that: * vprintf() does locking and __STDIO_STREAM_TRANS_TO_WRITE thing, then calls vprintf_internal_unlocked(); * vsnprintf, vdprintf.c, vasprintf.c all use vprintf_internal_unlocked() directly [and do not need to set up fake locking now]

(0006294) vda 04-01-08 14:50	2.patch implements this solution. 3.patch removes unneeded initialization of locking. If you are going to apply it, take a look at vsnprintf.c's comment about __bufputc - it seems to be bogus, AFAICS __bufputc initialization has nothing to do with __user_locking. (I did grep around). Didn't look yet whether it works with old_vfprintf.c.

(0006304) vda 04-01-08 15:22	These patches save Note: 0000400 bytes compared to unpatched uclibc. Next target can be removing __stdio_fwrite + __stdio_WRITE, as they should not be needed for sprintf too - projected to save Note: 0000300 bytes.

(0006454) vda 04-08-08 17:54	v2 of patches should be more correct wrt old_vsprintf (it is just left untouched now)

(0006494) vda 04-09-08 12:52	Applied as revisions 21683 and 21684.

Relationships