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  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

cmd: Avoid mixing signed and unsigned type in conditional expression

[Frédéric Delanoy]

On Thu, Oct 27, 2011 at 08:38, Eric Pouech  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 :
> On Thu, Oct 27, 2011 at 08:38, Eric Pouech  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  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 :
> On 10/27/2011 02:09 PM, Frédéric Delanoy wrote:
> > On Thu, Oct 27, 2011 at 08:38, Eric Pouech  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)...