Re: [PATCH] server_ioctl_file() - Prefix printed hex values with '0x'

[Alexandre Julliard]

Chris Morgan  writes:
> Avoids the ambiguity with non-alphanumeric hex values.
> Signed-off-by: Chris Morgan
>
> dlls/ntdll/file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c
> index fd7f3dd..4d081b7 100644
> --- a/dlls/ntdll/file.c
> +++ b/dlls/ntdll/file.c
> @@ -1567,7 +1567,7 @@ static NTSTATUS server_ioctl_file( HANDLE handle, HANDLE event,
>      SERVER_END_REQ;
>  if (status == STATUS_NOT_SUPPORTED)
> -
>    FIXME("Unsupported ioctl %x (device=%x access=%x func=%x method=%x)\n",
> -
>    FIXME("Unsupported ioctl 0x%x (device=0x%x access=0x%x func=0x%x method=0x%x)\n",

That's mostly a matter of taste, so I don't think it's necessary to
change it.

[PATCH] server_ioctl_file() - Prefix printed hex values with '0x'

[Chris Morgan]

On Fri, Mar 3, 2017 at 2:31 PM Alexandre Julliard 
wrote:
> Chris Morgan  writes:
> > Avoids the ambiguity with non-alphanumeric hex values.
> > Signed-off-by: Chris Morgan
> >
> > dlls/ntdll/file.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c
> > index fd7f3dd..4d081b7 100644
> > --- a/dlls/ntdll/file.c
> > +++ b/dlls/ntdll/file.c
> > @@ -1567,7 +1567,7 @@ static NTSTATUS server_ioctl_file( HANDLE handle,
>
> HANDLE event,
> >  SERVER_END_REQ;
> >
> >  if (status == STATUS_NOT_SUPPORTED)
> > -
> >    FIXME("Unsupported ioctl %x (device=%x access=%x func=%x
>
> method=%x)\n",
> > -
> >    FIXME("Unsupported ioctl 0x%x (device=0x%x access=0x%x
>
> func=0x%x method=0x%x)\n",
>
> That's mostly a matter of taste, so I don't think it's necessary to
> change it.
>
> --
> Alexandre Julliard
> [e-mail]

Ok. I think most of the rest of the file uses 0x and it cost me an hour or
so before I figured out they weren't decimal values as they didn't include
any letters. And the list of FSCTL options in headers uses decimal so that
was another layer of confusion.

There is one more case that doesn't use 0x that I found after submitting
the patch.

I'd still like to advocate for being explicit because it isn't always
obvious what base is used.

Chris

[PATCH] server_ioctl_file() - Prefix printed hex values with '0x'

[Alex Henrie]

2017-03-03 12:35 GMT-07:00 Chris Morgan :
> On Fri, Mar 3, 2017 at 2:31 PM Alexandre Julliard 
> wrote:
> > Chris Morgan  writes:
> > > Avoids the ambiguity with non-alphanumeric hex values.
> > > Signed-off-by: Chris Morgan
> > >
> > > dlls/ntdll/file.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c
> > > index fd7f3dd..4d081b7 100644
> > > --- a/dlls/ntdll/file.c
> > > +++ b/dlls/ntdll/file.c
> > > @@ -1567,7 +1567,7 @@ static NTSTATUS server_ioctl_file( HANDLE handle,
> > > HANDLE event,
> > >      SERVER_END_REQ;
> > >  if (status == STATUS_NOT_SUPPORTED)
> > > -
> > >    FIXME("Unsupported ioctl %x (device=%x access=%x func=%x
> > >
> > > method=%x)\n",
> > > -
> > >    FIXME("Unsupported ioctl 0x%x (device=0x%x access=0x%x
> > >
> > > func=0x%x method=0x%x)\n",
> >
> > That's mostly a matter of taste, so I don't think it's necessary to
> > change it.
> >
> > --
> > Alexandre Julliard
> > [e-mail]
>
> Ok. I think most of the rest of the file uses 0x and it cost me an hour or
> so before I figured out they weren't decimal values as they didn't include
> any letters. And the list of FSCTL options in headers uses decimal so that
> was another layer of confusion.
>
> There is one more case that doesn't use 0x that I found after submitting the
> patch.
>
> I'd still like to advocate for being explicit because it isn't always
> obvious what base is used.

I came across this old email cleaning out my inbox today, and I just
wanted to comment that I also think the 0x prefix is helpful. The only
exception that comes to mind is if the number is zero-padded (i.e.
%08x, %04x, or %02x). If the number is not zero-padded and does not
have a 0x prefix, I would expect it to be a decimal number.

-Alex