[PATCH] Remove extra 0x in the %p formatting string

[Timur I. Bakeyev via samba-technical]

Hi Timur,

Hi all!
While analyzing debug logs of our customer, I've noticed strange looking
[2018/05/09 16:02:52.629315, 10, pid=907, effective(1001, 1001), real(0,
0)] ../source3/smbd/open.c:5543(create_file_default)
create_file: access_mask = 0x100080 file_attributes = 0x10,

share_access

= 0x7, create_disposition = 0x1 create_options = 0x1 oplock_request = 0x0
private_flags = 0x0 root_dir_fid = 0x0, ea_list = 0x0x0, sd = 0x0x0,

fname

= .
That 0x0x0 puzzled me and short investigation showed that in few places

we

use "0x%p" format string to represent buffer address in the debug logs.

Man

p The void * pointer argument is printed in hexadecimal

(as

if
by ‘%#x’ or ‘%#lx’).
I.e. the address will be already prefixed with the 0x, no need to
explicitly add it.
Here is a small patch.

Looks good.
For the changes in lib/dbwrap/dbwrap_tdb.c and source3/smbd/open.c, can
you please modernise the debug statements that you touch by changing
them as below?
DEBUG(10, (...));
to
DBG_DEBUG(...);
Thanks...

Thanks for the review, Martin!

Here are the patches that should fit you requirements, I hope.

List, can I get a second review, please :)?

With regards,
Timur Bakeyev.

[Ralph Böhme via samba-technical]

Post by Timur I. Bakeyev via samba-technical
List, can I get a second review, please :)?

lgtm. Pushed to autobuild.

Fwiw, can you send patchsets as git format-patch --stdout instead of one
attachment per patch? Thanks!

-slow

[Franz Sirl via samba-technical]

Post by Timur I. Bakeyev via samba-technical

Hi Timur,

Hi all!
While analyzing debug logs of our customer, I've noticed strange looking
[2018/05/09 16:02:52.629315, 10, pid=907, effective(1001, 1001), real(0,
0)] ../source3/smbd/open.c:5543(create_file_default)
create_file: access_mask = 0x100080 file_attributes = 0x10,

share_access

= 0x7, create_disposition = 0x1 create_options = 0x1 oplock_request = 0x0
private_flags = 0x0 root_dir_fid = 0x0, ea_list = 0x0x0, sd = 0x0x0,

fname

= .
That 0x0x0 puzzled me and short investigation showed that in few places

we

use "0x%p" format string to represent buffer address in the debug logs.

Man

p The void * pointer argument is printed in hexadecimal

(as

if
by ‘%#x’ or ‘%#lx’).
I.e. the address will be already prefixed with the 0x, no need to
explicitly add it.
Here is a small patch.

Looks good.
For the changes in lib/dbwrap/dbwrap_tdb.c and source3/smbd/open.c, can
you please modernise the debug statements that you touch by changing
them as below?
DEBUG(10, (...));
to
DBG_DEBUG(...);
Thanks...

Thanks for the review, Martin!
Here are the patches that should fit you requirements, I hope.
List, can I get a second review, please :)?

Note that %p is not portable, for example Solaris 10/11 don't prefix 0x
to the value. And glibc prints "(nil)" for a NULL-pointer.

Franz.

[Timur I. Bakeyev via samba-technical]

Note that %p is not portable, for example Solaris 10/11 don't prefix 0x to
the value. And glibc prints "(nil)" for a NULL-pointer.

I believe that %p is in the ANSI C standard. Solaris with it's own very
special C compiler.. Unfortunately, standard doesn't really define, how the
pointer should look like. Hence lack of 0x in some implementations and
(nil). As for the last one, I believe that

buffer is: (nil)

still looks better than:

buffer is: 0x(nil)

If we want even more portability we may employ PRIxPTR macro from
stdint.h/inttypes.h(which didn't present in Solaris 2.8 at least :>) and
cast to uintptr_t. But not sure we need such a complications.

With regards,
Timur Bakeyev.

[Andrew Bartlett via samba-technical]

On Wed, 2018-05-16 at 18:18 +0200, Franz Sirl via samba-technical

Post by Franz Sirl via samba-technical
Note that %p is not portable, for example Solaris 10/11 don't prefix 0x
to the value. And glibc prints "(nil)" for a NULL-pointer.

A patch to lib/replace/tests/snprintf.c would be a good idea then. We
can then replace printf on these platforms.

Andrew Bartlett