diff mbox

argp: Use fwrite_unlocked instead of __fxprintf when !_LIBC

Message ID 1451679626-28351-1-git-send-email-raj.khem@gmail.com
State New, archived
Headers

Commit Message

Khem Raj Jan. 1, 2016, 8:20 p.m. UTC 
  __fxprintf is not available when argp is built outside libc

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 ChangeLog             | 5 +++++
 argp/argp-fmtstream.c | 4 ++++
 2 files changed, 9 insertions(+)

Comments

Mike Frysinger Jan. 2, 2016, 3:21 a.m. UTC | #1 
On 01 Jan 2016 20:20, Khem Raj wrote:
> __fxprintf is not available when argp is built outside libc
> 
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---
>  ChangeLog             | 5 +++++
>  argp/argp-fmtstream.c | 4 ++++

i forget where we document these things, but usually modules that utilize
_LIBC are kept in sync with other projects, and we often like to see such
changes sent/merged simultaneously with them.  this one looks like gnulib.
-mike
Florian Weimer Jan. 4, 2016, 7:10 a.m. UTC | #2 
On 01/01/2016 09:20 PM, Khem Raj wrote:

> +#ifdef _LIBC
>        __fxprintf (fs->stream, "%.*s", (int) (fs->p - fs->buf), fs->buf);
> +#else
> +      fwrite_unlocked (fs->buf, 1, fs->p - fs->buf, fs->stream);
> +#endif

Why not use fwrite_unlocked unconditionally?

Thanks,
Florian
Joseph Myers Jan. 4, 2016, 3:36 p.m. UTC | #3 
On Mon, 4 Jan 2016, Florian Weimer wrote:

> On 01/01/2016 09:20 PM, Khem Raj wrote:
> 
> > +#ifdef _LIBC
> >        __fxprintf (fs->stream, "%.*s", (int) (fs->p - fs->buf), fs->buf);
> > +#else
> > +      fwrite_unlocked (fs->buf, 1, fs->p - fs->buf, fs->stream);
> > +#endif
> 
> Why not use fwrite_unlocked unconditionally?

Does fwrite_unlocked work on wide-oriented streams in glibc (the point of 
__fxprintf as I understand it being to work whatever the orientation of 
the stream)?
Khem Raj Jan. 4, 2016, 7:08 p.m. UTC | #4 
On Fri, Jan 1, 2016 at 7:21 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On 01 Jan 2016 20:20, Khem Raj wrote:
>> __fxprintf is not available when argp is built outside libc
>>
>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
>> ---
>>  ChangeLog             | 5 +++++
>>  argp/argp-fmtstream.c | 4 ++++
>
> i forget where we document these things, but usually modules that utilize
> _LIBC are kept in sync with other projects, and we often like to see such
> changes sent/merged simultaneously with them.  this one looks like gnulib.

gnulib already have a check for it introduced via this commit

http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=c72047a1ed44c8c9509f1593d726ccd63ceb965c

it checks for USE_IN_LIBIO

> -mike
Mike Frysinger Jan. 4, 2016, 7:31 p.m. UTC | #5 
On 04 Jan 2016 11:08, Khem Raj wrote:
> On Fri, Jan 1, 2016 at 7:21 PM, Mike Frysinger wrote:
> > On 01 Jan 2016 20:20, Khem Raj wrote:
> >> __fxprintf is not available when argp is built outside libc
> >>
> >> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> >> ---
> >>  ChangeLog             | 5 +++++
> >>  argp/argp-fmtstream.c | 4 ++++
> >
> > i forget where we document these things, but usually modules that utilize
> > _LIBC are kept in sync with other projects, and we often like to see such
> > changes sent/merged simultaneously with them.  this one looks like gnulib.
> 
> gnulib already have a check for it introduced via this commit
> 
> http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=c72047a1ed44c8c9509f1593d726ccd63ceb965c

that's stronger justification imo for merging

> it checks for USE_IN_LIBIO

it would be nice if we could fully sync these two files so they're
in agreement again, but i won't force that on you :).
-mike
Florian Weimer Jan. 4, 2016, 7:38 p.m. UTC | #6 
On 01/04/2016 04:36 PM, Joseph Myers wrote:
> On Mon, 4 Jan 2016, Florian Weimer wrote:
> 
>> On 01/01/2016 09:20 PM, Khem Raj wrote:
>>
>>> +#ifdef _LIBC
>>>        __fxprintf (fs->stream, "%.*s", (int) (fs->p - fs->buf), fs->buf);
>>> +#else
>>> +      fwrite_unlocked (fs->buf, 1, fs->p - fs->buf, fs->stream);
>>> +#endif
>>
>> Why not use fwrite_unlocked unconditionally?
> 
> Does fwrite_unlocked work on wide-oriented streams in glibc (the point of 
> __fxprintf as I understand it being to work whatever the orientation of 
> the stream)?

Oh, I didn't know this was the point of fxprintf.  No, in this case,
using fwrite_unlocked would not work (in fact, it would be undefined).

Florian
Khem Raj Jan. 5, 2016, 9:13 p.m. UTC | #7 
> On Jan 4, 2016, at 11:31 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> 
> On 04 Jan 2016 11:08, Khem Raj wrote:
>> On Fri, Jan 1, 2016 at 7:21 PM, Mike Frysinger wrote:
>>> On 01 Jan 2016 20:20, Khem Raj wrote:
>>>> __fxprintf is not available when argp is built outside libc
>>>> 
>>>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
>>>> ---
>>>> ChangeLog             | 5 +++++
>>>> argp/argp-fmtstream.c | 4 ++++
>>> 
>>> i forget where we document these things, but usually modules that utilize
>>> _LIBC are kept in sync with other projects, and we often like to see such
>>> changes sent/merged simultaneously with them.  this one looks like gnulib.
>> 
>> gnulib already have a check for it introduced via this commit
>> 
>> http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=c72047a1ed44c8c9509f1593d726ccd63ceb965c
> 
> that's stronger justification imo for merging


Can you help with merging it

> 
>> it checks for USE_IN_LIBIO
> 
> it would be nice if we could fully sync these two files so they're
> in agreement again, but i won't force that on you :).

USE_IN_LIBIO is no longer used in glibc.

> -mike
Mike Frysinger Jan. 7, 2016, 9:28 a.m. UTC | #8 
On 05 Jan 2016 13:13, Khem Raj wrote:
> > On Jan 4, 2016, at 11:31 AM, Mike Frysinger wrote:
> > On 04 Jan 2016 11:08, Khem Raj wrote:
> >> On Fri, Jan 1, 2016 at 7:21 PM, Mike Frysinger wrote:
> >>> On 01 Jan 2016 20:20, Khem Raj wrote:
> >>>> __fxprintf is not available when argp is built outside libc
> >>>> 
> >>>> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> >>>> ---
> >>>> ChangeLog             | 5 +++++
> >>>> argp/argp-fmtstream.c | 4 ++++
> >>> 
> >>> i forget where we document these things, but usually modules that utilize
> >>> _LIBC are kept in sync with other projects, and we often like to see such
> >>> changes sent/merged simultaneously with them.  this one looks like gnulib.
> >> 
> >> gnulib already have a check for it introduced via this commit
> >> 
> >> http://git.savannah.gnu.org/cgit/gnulib.git/commit/?id=c72047a1ed44c8c9509f1593d726ccd63ceb965c
> > 
> > that's stronger justification imo for merging
> 
> Can you help with merging it

pushed now

> >> it checks for USE_IN_LIBIO
> > 
> > it would be nice if we could fully sync these two files so they're
> > in agreement again, but i won't force that on you :).
> 
> USE_IN_LIBIO is no longer used in glibc.

i don't mean the code needs to flow in one direction
-mike
diff mbox

Patch

diff --git a/ChangeLog b/ChangeLog
index 51a055c..0bfb1fd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@ 
+2016-01-01  Khem Raj  <raj.khem@gmail.com>
+
+	* argp/argp-fmtstream.c (__argp_fmtstream_free): Use fwrite_unlocked
+	instead of __fxprintf when _LIBC is undefined.
+
 2015-12-30  Dmitry V. Levin  <ldv@altlinux.org>
 
 	[BZ #19408]
diff --git a/argp/argp-fmtstream.c b/argp/argp-fmtstream.c
index 2b845e0..f8de40e 100644
--- a/argp/argp-fmtstream.c
+++ b/argp/argp-fmtstream.c
@@ -100,7 +100,11 @@  __argp_fmtstream_free (argp_fmtstream_t fs)
   __argp_fmtstream_update (fs);
   if (fs->p > fs->buf)
     {
+#ifdef _LIBC
       __fxprintf (fs->stream, "%.*s", (int) (fs->p - fs->buf), fs->buf);
+#else
+      fwrite_unlocked (fs->buf, 1, fs->p - fs->buf, fs->stream);
+#endif
     }
   free (fs->buf);
   free (fs);