Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++ compile has warnings #23

Closed
parsley72 opened this issue Dec 7, 2017 · 1 comment
Closed

C++ compile has warnings #23

parsley72 opened this issue Dec 7, 2017 · 1 comment

Comments

@parsley72
Copy link

parsley72 commented Dec 7, 2017

When I compile the C++ version I get:

qr-code-generator\qrcode.cpp(429): warning C4701: potentially uninitialized local variable 'colorX' used
qr-code-generator\qrcode.cpp(433): warning C4701: potentially uninitialized local variable 'runX' used
qr-code-generator\qrcode.cpp(445): warning C4701: potentially uninitialized local variable 'colorY' used
qr-code-generator\qrcode.cpp(449): warning C4701: potentially uninitialized local variable 'runY' used

Also:

QR-Code-generator/BitBuffer.cpp: In member function 'std::vector<unsigned char> qrcodegen::BitBuffer::getBytes() const':
QR-Code-generator/BitBuffer.cpp:39:24: error: conversion to '__gnu_cxx::__alloc_traits<std::allocator<unsigned char> >::value_type {aka unsigned char}' from 'int' may alter its value [-Werror=conversion]
         result[i >> 3] |= (*this)[i] ? 1 << (7 - (i & 7)) : 0;
                        ^
QR-Code-generator/QrCode.cpp: In constructor 'qrcodegen::QrCode::ReedSolomonGenerator::ReedSolomonGenerator(int)':
QR-Code-generator/QrCode.cpp:696:36: error: conversion to '__gnu_cxx::__alloc_traits<std::allocator<unsigned char> >::value_type {aka unsigned char}' from 'int' may alter its value [-Werror=conversion]
                 coefficients.at(j) ^= coefficients.at(j + 1);
                                    ^
QR-Code-generator/QrCode.cpp: In member function 'std::vector<unsigned char> qrcodegen::QrCode::ReedSolomonGenerator::getRemainder(const std::vector<unsigned char>&) const':
QR-Code-generator/QrCode.cpp:713:26: error: conversion to '__gnu_cxx::__alloc_traits<std::allocator<unsigned char> >::value_type {aka unsigned char}' from 'int' may alter its value [-Werror=conversion]
             result.at(j) ^= multiply(coefficients.at(j), factor);

@nayuki
Copy link
Owner

nayuki commented Dec 10, 2017

You should have stated which compiler version you are using; it would help me understand why you are getting these warning messages. From the provided text, it looks like you are using Visual C++ on Windows in the first block, and GCC on Linux in the second block. Your line numbers (e.g. BitBuffer.cpp:39, QrCode.cpp:696) are different from my reference copy. I had to use the quoted text to find the code in question. Please try to be more helpful in future bug reports, not just to me, but to anyone.

I fixed your first set of warnings in 908dbbf . I did some testing of the logic a few months ago, and found that GCC and Clang/LLVM are smart enough to understand that the 0th iteration of the loop will necessarily overwrite colorX, runX, etc. Thus it is mathematically impossible to have an uninitialized read of the variables. If I change the condition to x == 1, then GCC and LLVM correctly figure out that the variable may (will) be uninitialized before reading. I believe Visual C++ is not smart enough to figure this out, so it always gives the warning.

For the second set of warnings, I have never seen them in any compiler I used. My overall verdict is will-not-fix, because the warnings are overly pedantic (which I have not even seen using -Wpedantic on my compilers!), and the fix would make the code ugly. The crux of the three conversion warnings is the interplay between uint_8 (unsigned char) and int (int16_t or wider).

  • The first warning ("BitBuffer.cpp:39") basically concerns: uint8_t |= (bool ? int : int);. However, the right side value always fits in a uint8_t.
  • The second and third warnings ("QrCode.cpp:696,713") basically concern: uin8_t ^= uint8_t; The result always fits in a uint8_t. However, the expression expands to uint8_t = uint8_t ^ uint8_t, where the right side is promoted to int. This would mean I would need to write out uint8_t = static_cast<uint8_t>(uint8_t ^ uint8_t);, which only adds to the obfuscation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants