[PATCH 1/1] ungetc: Cast to unsigned char before push back to stream

[PATCH 1/1] ungetc: Cast to unsigned char before push back to stream

[wangjianjian (C)]

From 4d24e6fee85ed878dc632dd29aabeb7c7454952e Mon Sep 17 00:00:00 2001
From: Wang Jianjian <wangjianjian3@huawei.com>
Date: Fri, 18 Oct 2019 20:28:29 +0800
Subject: [PATCH 1/1] ungetc: Cast to unsigned char before push back to stream

Per Posix standard, casting to unsigned char is need before returning C
and pushing C back to stream.

Signed-off-by: Wang Jianjian <wangjianjian3@huawei.com>
---
 src/stdio/ungetc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/stdio/ungetc.c b/src/stdio/ungetc.c
index 180673a4..9733091a 100644
--- a/src/stdio/ungetc.c
+++ b/src/stdio/ungetc.c
@@ -12,9 +12,9 @@ int ungetc(int c, FILE *f)
                return EOF;
        }

-       *--f->rpos = c;
+       *--f->rpos = (unsigned char)c;
        f->flags &= ~F_EOF;

        FUNLOCK(f);
-       return c;
+       return (unsigned char)c;
 }
--
2.17.1

BR,
Wang Jianjian

Re: [PATCH 1/1] ungetc: Cast to unsigned char before push back to stream

[Rich Felker]

On Fri, Oct 18, 2019 at 12:35:45PM +0000, wangjianjian (C) wrote:
> >From 4d24e6fee85ed878dc632dd29aabeb7c7454952e Mon Sep 17 00:00:00 2001
> From: Wang Jianjian <wangjianjian3@huawei.com>
> Date: Fri, 18 Oct 2019 20:28:29 +0800
> Subject: [PATCH 1/1] ungetc: Cast to unsigned char before push back to stream
> 
> Per Posix standard, casting to unsigned char is need before returning C
> and pushing C back to stream.
> 
> Signed-off-by: Wang Jianjian <wangjianjian3@huawei.com>
> ---
>  src/stdio/ungetc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/stdio/ungetc.c b/src/stdio/ungetc.c
> index 180673a4..9733091a 100644
> --- a/src/stdio/ungetc.c
> +++ b/src/stdio/ungetc.c
> @@ -12,9 +12,9 @@ int ungetc(int c, FILE *f)
>                 return EOF;
>         }
> 
> -       *--f->rpos = c;
> +       *--f->rpos = (unsigned char)c;

This line does not change anything.

>         f->flags &= ~F_EOF;
> 
>         FUNLOCK(f);
> -       return c;
> +       return (unsigned char)c;
>  }
> --
> 2.17.1

I believe this is actually a functional change, and a needed one for
conformance (to ISO C, not specific to POSIX). The issue it seems to
solve, which is what the change needs to be documented as, is the
return value when a negative value not equal to EOF is passed to
ungetc. In this case, the right value is already pushed back, but the
function wrongly returns the original negative value passed in rather
than the value that was pushed back.

Rich

Re: [PATCH 1/1] ungetc: Cast to unsigned char before push back to stream

[Rich Felker]

On Fri, Oct 18, 2019 at 09:11:49AM -0400, Rich Felker wrote:
> On Fri, Oct 18, 2019 at 12:35:45PM +0000, wangjianjian (C) wrote:
> > >From 4d24e6fee85ed878dc632dd29aabeb7c7454952e Mon Sep 17 00:00:00 2001
> > From: Wang Jianjian <wangjianjian3@huawei.com>
> > Date: Fri, 18 Oct 2019 20:28:29 +0800
> > Subject: [PATCH 1/1] ungetc: Cast to unsigned char before push back to stream
> > 
> > Per Posix standard, casting to unsigned char is need before returning C
> > and pushing C back to stream.
> > 
> > Signed-off-by: Wang Jianjian <wangjianjian3@huawei.com>
> > ---
> >  src/stdio/ungetc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/stdio/ungetc.c b/src/stdio/ungetc.c
> > index 180673a4..9733091a 100644
> > --- a/src/stdio/ungetc.c
> > +++ b/src/stdio/ungetc.c
> > @@ -12,9 +12,9 @@ int ungetc(int c, FILE *f)
> >                 return EOF;
> >         }
> > 
> > -       *--f->rpos = c;
> > +       *--f->rpos = (unsigned char)c;
> 
> This line does not change anything.
> 
> >         f->flags &= ~F_EOF;
> > 
> >         FUNLOCK(f);
> > -       return c;
> > +       return (unsigned char)c;
> >  }
> > --
> > 2.17.1
> 
> I believe this is actually a functional change, and a needed one for
> conformance (to ISO C, not specific to POSIX). The issue it seems to
> solve, which is what the change needs to be documented as, is the
> return value when a negative value not equal to EOF is passed to
> ungetc. In this case, the right value is already pushed back, but the
> function wrongly returns the original negative value passed in rather
> than the value that was pushed back.

Committing simplified version with just the second change. Thanks for
catching this.

Rich

Re: [PATCH 1/1] ungetc: Cast to unsigned char before push back to stream

[Szabolcs Nagy]

* wangjianjian (C) <wangjianjian3@huawei.com> [2019-10-18 12:35:45 +0000]:
> >From 4d24e6fee85ed878dc632dd29aabeb7c7454952e Mon Sep 17 00:00:00 2001
> From: Wang Jianjian <wangjianjian3@huawei.com>
> Date: Fri, 18 Oct 2019 20:28:29 +0800
> Subject: [PATCH 1/1] ungetc: Cast to unsigned char before push back to stream
> 
> Per Posix standard, casting to unsigned char is need before returning C
> and pushing C back to stream.
> 
> Signed-off-by: Wang Jianjian <wangjianjian3@huawei.com>
> ---
>  src/stdio/ungetc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/stdio/ungetc.c b/src/stdio/ungetc.c
> index 180673a4..9733091a 100644
> --- a/src/stdio/ungetc.c
> +++ b/src/stdio/ungetc.c
> @@ -12,9 +12,9 @@ int ungetc(int c, FILE *f)
>                 return EOF;
>         }
> 
> -       *--f->rpos = c;
> +       *--f->rpos = (unsigned char)c;

this is a noop since assignment will do the conversion.

>         f->flags &= ~F_EOF;
> 
>         FUNLOCK(f);
> -       return c;
> +       return (unsigned char)c;

i think this fixes a real bug.

>  }
> --
> 2.17.1
> 
> BR,
> Wang Jianjian

答复: [musl] [PATCH 1/1] ungetc: Cast to unsigned char before push back to stream

[wangjianjian (C)]

Thanks for review. I will send a v2.

-----邮件原件-----
发件人: Szabolcs Nagy [mailto:nsz@port70.net] 
发送时间: 2019年10月18日 21:15
收件人: musl@lists.openwall.com
抄送: wangjianjian (C) <wangjianjian3@huawei.com>
主题: Re: [musl] [PATCH 1/1] ungetc: Cast to unsigned char before push back to stream

* wangjianjian (C) <wangjianjian3@huawei.com> [2019-10-18 12:35:45 +0000]:
> >From 4d24e6fee85ed878dc632dd29aabeb7c7454952e Mon Sep 17 00:00:00 
> >2001
> From: Wang Jianjian <wangjianjian3@huawei.com>
> Date: Fri, 18 Oct 2019 20:28:29 +0800
> Subject: [PATCH 1/1] ungetc: Cast to unsigned char before push back to 
> stream
> 
> Per Posix standard, casting to unsigned char is need before returning 
> C and pushing C back to stream.
> 
> Signed-off-by: Wang Jianjian <wangjianjian3@huawei.com>
> ---
>  src/stdio/ungetc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/stdio/ungetc.c b/src/stdio/ungetc.c index 
> 180673a4..9733091a 100644
> --- a/src/stdio/ungetc.c
> +++ b/src/stdio/ungetc.c
> @@ -12,9 +12,9 @@ int ungetc(int c, FILE *f)
>                 return EOF;
>         }
> 
> -       *--f->rpos = c;
> +       *--f->rpos = (unsigned char)c;

this is a noop since assignment will do the conversion.

>         f->flags &= ~F_EOF;
> 
>         FUNLOCK(f);
> -       return c;
> +       return (unsigned char)c;

i think this fixes a real bug.

>  }
> --
> 2.17.1
> 
> BR,
> Wang Jianjian