libqrencode icon indicating copy to clipboard operation
libqrencode copied to clipboard

int or size_t, which is better?

Open fukuchi opened this issue 8 years ago • 4 comments

Basically, libqrencode uses 'int' instead of 'size_t' type for size attributes. While this worked without any problems, clang warns like following:

$ CC=clang CFLAGS="-Wshorten-64-to-32 -Wsign-conversion" ./configure --with-tests
$ make
(snipped)
qrencode.c:127:46: warning: implicit conversion changes signedness: 'int' to
      'unsigned long' [-Wsign-conversion]
        raw->ecccode = (unsigned char *)malloc(raw->eccLength);
                                        ~~~~~~ ~~~~~^~~~~~~~~
(snipped)

Typecasting is an easy solution, but how about this?

qrenc.c:204:12: warning: implicit conversion loses integer precision:
      'unsigned long' to 'int' [-Wshorten-64-to-32]
        int len = strlen(value);
            ~~~   ^~~~~~~~~~~~~

Not only to suppress the warning messages, but also to reduce security risks, we should take care of this warnings.

fukuchi avatar Nov 19 '16 14:11 fukuchi

I don't know if this question is still relevant or useful to you, but if I remember correctly, size_t produces more predictable behavior when writing for cross platform compatibility.

I don't know if that's a concern for libqrencode, but I've seen it as the winning argument between the two in other C projects.

mashiox avatar Apr 27 '17 22:04 mashiox

I have added a Visual Studio 2015 project to create a static library of libqrencode V3.4.4 for use in an Open Source project. All works fine (thank you).

However, there are 25 warnings in the x64 build that are not in the x86 build and this is related to the int/size_t, __int64 and int and similar.

One warning is because the POSIX function 'strdup' is depreciated and should be '_strdup' and 3 relate to unreferenced formal parameters.

Some issues mentioning size_t are because, when the library gets the size of a string using 'strlen', it returns a size_t value but the result is used as an integer in calling another function. However, this is not the whole cause..

I do not like warnings and nor keen on casting but the changes are more than this.

I would be grateful if you would fix these (without just suppressing the warning messages!).

The warning messages are below (the last number is the line number in that source member):

Warning C4244 'function': conversion from '__int64' to 'int', possible loss of data ..\split.c 220 Warning C4244 '=': conversion from 'int' to 'data_t', possible loss of data ..\rscode.c 151 Warning C4244 '=': conversion from 'int' to 'data_t', possible loss of data ..\rscode.c 155 Warning C4244 '=': conversion from 'int' to 'data_t', possible loss of data ..\rscode.c 156 Warning C4244 '=': conversion from '__int64' to 'int', possible loss of data ..\split.c 98 Warning C4244 '=': conversion from '__int64' to 'int', possible loss of data ..\split.c 155 Warning C4244 '=': conversion from '__int64' to 'int', possible loss of data ..\split.c 182 Warning C4244 '=': conversion from '__int64' to 'int', possible loss of data ..\split.c 252 Warning C4100 'data': unreferenced formal parameter ..\qrinput.c 765 Warning C4244 'function': conversion from 'int' to 'unsigned char', possible loss of data ..\qrinput.c 1693 Warning C4267 'function': conversion from 'size_t' to 'int', possible loss of data ..\qrencode.c 721 Warning C4267 'function': conversion from 'size_t' to 'int', possible loss of data ..\qrencode.c 735 Warning C4267 'function': conversion from 'size_t' to 'int', possible loss of data ..\qrencode.c 893 Warning C4267 'function': conversion from 'size_t' to 'int', possible loss of data ..\qrencode.c 902 Warning C4244 'function': conversion from '__int64' to 'int', possible loss of data ..\split.c 141 Warning C4244 'function': conversion from '__int64' to 'int', possible loss of data ..\split.c 142 Warning C4244 'function': conversion from '__int64' to 'int', possible loss of data ..\split.c 144 Warning C4244 'function': conversion from '__int64' to 'int', possible loss of data ..\split.c 219 Warning C4244 'function': conversion from '__int64' to 'int', possible loss of data ..\split.c 222 Warning C4244 'function': conversion from '__int64' to 'int', possible loss of data ..\split.c 238 Warning C4244 'function': conversion from '__int64' to 'int', possible loss of data ..\split.c 239 Warning C4244 'function': conversion from '__int64' to 'int', possible loss of data ..\split.c 241 Warning C4996 'strdup': The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name: _strdup. See online help for details. ..\split.c 287 Warning C4100 'version': unreferenced formal parameter ..\qrinput.c 772 Warning C4100 'version': unreferenced formal parameter ..\qrinput.c 825

c-273 avatar Aug 18 '17 10:08 c-273

Thank you for your comments (and kicking my ass), @mashiox and @c-273. Soon I'll write some test code then improve the warned lines.

fukuchi avatar Aug 19 '17 15:08 fukuchi