Re: cmd: Avoid mixing signed and unsigned type in conditional expression

[Eric Pouech]

why do we check for ptr being null or not, when we deref ptr one line above
?
something's wrong in the code logic
A+

Le 27 octobre 2011 04:53, Frédéric Delanoy <frederic.delanoy(a)gmail.com> a
écrit :
> ---
>  programs/cmd/wcmdmain.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
> index 56aa85f..1bcc598 100644
> --- a/programs/cmd/wcmdmain.c
> +++ b/programs/cmd/wcmdmain.c
> @@ -278,7 +278,7 @@ static void WCMD_output_asis_handle (DWORD std_handle,
> const WCHAR *message) {
>         ptr++;
>       };
>       if (*ptr == '\n') ptr++;
> -      WCMD_output_asis_len(message, (ptr) ? ptr - message :
> strlenW(message), handle);
> +      WCMD_output_asis_len(message, (ptr) ? (DWORD)(ptr - message) :
> strlenW(message), handle);
>       if (ptr) {
>         numChars = 0;
>         if (++line_count >= max_height - 1) {
> --
> 1.7.7.1

-- 
-- 
Eric Pouech

cmd: Avoid mixing signed and unsigned type in conditional expression

[Frédéric Delanoy]

On Thu, Oct 27, 2011 at 08:38, Eric Pouech <eric.pouech(a)orange.fr> wrote:
> why do we check for ptr being null or not, when we deref ptr one line above?
> >       if (*ptr == '\n') ptr++;
> > -      WCMD_output_asis_len(message, (ptr) ? ptr - message :
> > strlenW(message), handle);
> > +      WCMD_output_asis_len(message, (ptr) ? (DWORD)(ptr - message) :
> > strlenW(message), handle);
> >       if (ptr) {
> >         numChars = 0;
> >         if (++line_count >= max_height - 1) {

It's dereferenced but incremented afterwards, so might be null after that line

cmd: Avoid mixing signed and unsigned type in conditional expression

[Frédéric Delanoy]

2011/10/27 Frédéric Delanoy <frederic.delanoy(a)gmail.com>:
> On Thu, Oct 27, 2011 at 08:38, Eric Pouech <eric.pouech(a)orange.fr> wrote:
> > why do we check for ptr being null or not, when we deref ptr one line above?
> > >       if (*ptr == '\n') ptr++;
> > > -      WCMD_output_asis_len(message, (ptr) ? ptr - message :
> > > strlenW(message), handle);
> > > +      WCMD_output_asis_len(message, (ptr) ? (DWORD)(ptr - message) :
> > > strlenW(message), handle);
> > >       if (ptr) {
> > >         numChars = 0;
> > >         if (++line_count >= max_height - 1) {
>
> It's dereferenced but incremented afterwards, so might be null after that line

Of course if (*ptr != '\n') the check is unnecessary. I guess the
original author simply wanted to merge both cases. Might need some
reorganization, but that's probably outside of the scope of this patch

cmd: Avoid mixing signed and unsigned type in conditional expression

[Michael Stefaniuc]

On 10/27/2011 02:09 PM, Frédéric Delanoy wrote:
> On Thu, Oct 27, 2011 at 08:38, Eric Pouech <eric.pouech(a)orange.fr> wrote:
> > why do we check for ptr being null or not, when we deref ptr one line above?
> > > if (*ptr == '\n') ptr++;
> > > -      WCMD_output_asis_len(message, (ptr) ? ptr - message :
> > > strlenW(message), handle);
> > > +      WCMD_output_asis_len(message, (ptr) ? (DWORD)(ptr - message) :
> > > strlenW(message), handle);
> > >       if (ptr) {
> > >         numChars = 0;
> > >         if (++line_count >= max_height - 1) {
>
> It's dereferenced but incremented afterwards, so might be null after that line
> Only if ptr wraps around ;)

bye
	michael

cmd: Avoid mixing signed and unsigned type in conditional expression

[Frédéric Delanoy]

2011/10/27 Michael Stefaniuc <mstefani(a)redhat.com>:
> On 10/27/2011 02:09 PM, Frédéric Delanoy wrote:
> > On Thu, Oct 27, 2011 at 08:38, Eric Pouech <eric.pouech(a)orange.fr> wrote:
> > > why do we check for ptr being null or not, when we deref ptr one line above?
> > > >       if (*ptr == '\n') ptr++;
> > > > -      WCMD_output_asis_len(message, (ptr) ? ptr - message :
> > > > strlenW(message), handle);
> > > > +      WCMD_output_asis_len(message, (ptr) ? (DWORD)(ptr - message) :
> > > > strlenW(message), handle);
> > > >       if (ptr) {
> > > >         numChars = 0;
> > > >         if (++line_count >= max_height - 1) {
> >
> > It's dereferenced but incremented afterwards, so might be null after that line
> > Only if ptr wraps around ;)

Yeah right. Don't know where I saw a * before the ptr in (ptr)...