gisle / tcl.pm

Tcl extension module for Perl
http://search.cpan.org/dist/Tcl
9 stars 8 forks source link

Tcl.xs: use `%ld` for `count` in format strings #13

Closed chrstphrchvz closed 6 years ago

chrstphrchvz commented 6 years ago

continuing from https://github.com/gisle/tcl.pm/pull/12#issuecomment-398592805:

  1. cast occurrences of count to int when used in croak(); this hasn't been applied already. Nor did my compiler (clang) warn me about this.

My compiler didn't warn me likely because I32 is already an int (i.e. ints are 32 bits) on my system whereas the reporter of the issue might have had 16-bit ints.

To prevent the compiler warning, the approach I slightly prefer over casting count to int (as in the initially proposed patch) would be to instead use the %ld format specifier (where %ld is long which is at least 32 bits). Though I wouldn't expect something to use ~2**15 args anyways.

After merging this, ticket 78308 can probably be closed.

chrstphrchvz commented 6 years ago

Well, if the point was to avoid a compiler warning, then that didn't help much:

Tcl.xs:790:7: warning: format specifies type 'long' but the argument has type 'I32' (aka 'int') [-Wformat]
                    count);
                    ^~~~~
Tcl.xs:870:7: warning: format specifies type 'long' but the argument has type 'I32' (aka 'int') [-Wformat]
                    count);
                    ^~~~~

Meaning I have 64-bit longs. The other approach I would then try is using PRIi32 as the format specifier, but I'm not aware if inttypes.h can/should be used from XS code.

I will need to revise this before it can be merged.

chrstphrchvz commented 6 years ago

It doesn't look like there's official format strings for minimum-width types like I32. Since a long is always 32 bits or more, it should be enough to cast count to long and use %ld. (I've verified that the compiler warning goes away this time.)