3

Recently a developer at my company committed some code that looked something like this:

char buf[50];
string str;
str = sprintf(buf, "%s", "test");
//proceeds to use str

The thing is, it slipped through CI because the compiler raised no warnings despite -Wall and -Werror being set.

Shouldn't this be an obvious type mismatch? You can't assign an integer to an std::string type without std::to_string...

I took a look at the list of string assignments but I can't tell which one is being triggered in this case? Is it using one of these?

c-string (2) string& operator= (const char* s);
character (3) string& operator= (char c);

I'm guessing the latter, but that still seems like a compiler fail since sprintf clearly returns int not char.

Is there a warning we could have enabled that could have saved us in this case not covered by -Wall?

Edit:

A related thread I found: https://stackoverflow.com/a/39285668/2516916

Improve this question
5
  • Generates a warning for me. Is -Wimplicit-int-conversion included in -Wall? Commented Apr 23, 2020 at 2:20
  • It doesn't seem to matter which platform I choose. I've tried 32-bit, 64-bit, GCC, and clang, nothing generates a warning: repl.it/repls/DefinitiveVengefulBrowser Commented Apr 23, 2020 at 2:20
  • @Eljay which compiler version are you using? GCC 7 doesn't seem to have -Wimplicit-int-conversion Commented Apr 23, 2020 at 2:24
  • I use a heavily modified and instrumented version of clang. Commented Apr 23, 2020 at 2:29
  • This doesn't address the question, but this code does not initialize str with an int. It assigns an int to str which has already been initialized. It was initialized when it was created. Commented Apr 23, 2020 at 14:01

2 Answers 2

Reset to default
3

The return value of snprintf is an int which can be implicitely casted to a char therefore str = sprintf(buf, "%s", "test"); calls the character (3) string& operator= (char c); assignement operator.

The usual way of preventing that is to add the explicit keyword, but in your case since std::string is in the library you can't really do anything.

One way of detecting that is using UBSAN which has an option for implicit casts.

Hope that helps!

Improve this answer
Sign up to request clarification or add additional context in comments.

8 Comments

That seems like an evil thing for a compiler to do... char -> int I understand, but int is way bigger than char and information might be destroyed if you implicitly cast the other direction!
Unfortunately this is the case :( This qualifies as an implicit cast and is called a Numeric Conversion (see this).
There is no such thing as an implicit cast. A cast is something you write in your source code to tell the compiler to do a conversion. What you're seeing here is an implicit conversion.
@Gillespie -- yes, it's possible to write subtle rules that allow some conversions and not others based on the value involved. In fact, if I remember correctly, that's part of the rules for the "new" initialization syntax: char ch{0} is okay, but char ch{65535}; is not (unless 65535 can fit in a char).
Yes you can use -Wc++11-narrowing if you wanna detect wrong initilization
|
1

I made some tests and here is all my results. Compiling this program: 

#include <iostream>    
#include <string>

using namespace std;

int main ()
{
    string str;
    str = 1000;

    cout << str << "\n";
    return 0;
}

With the command:

g++ -o test test.cpp

you get the following warning:

teste.cpp: In function ‘int main()’:
teste.cpp:10:11: warning: overflow in implicit constant conversion [-Woverflow]
str = 1000;

if instead of 1000 you use a number from 0 to 255 then you do not get a warning. Apparently if the number is within the range of a char the compiler is trying to transform that number into a char and the string type accepts that.

But now if you do this:

#include <iostream>    
#include <string>

using namespace std;
int number(){
     return 100;
}

int main ()
{
    int a = 100;
    string str;
    str = a;

    cout << str << "\n";

    str = number();
    cout << str << "\n";

    return 0;
}

You do not get any warning and as output you get:

d
d

If instead of 100 you use 10000 in my case i got only two blank lines as result. So after that analysis i believe that what is happening is that even if what the variable str is trying to receive is a int The compiler is trying to make your life easier by converting everything to you, but unfortunatly in this case it made the exact opposite.

After some research here cpp-flags i finally found a combination that would have saved you:

g++ -o test test.cpp -Wconversion -Werror

with -Wconversion you turn those implicit conversions made by the compiler into warnings and with -Werror you turn every warning into an error.

and then in the program above you would get:

test.cpp: In function ‘int main()’:
test.cpp:13:11: error: conversion to ‘char’ from ‘int’ may alter its value [- 
Werror=conversion]
   str = a;
       ^
test.cpp:17:17: error: conversion to ‘char’ from ‘int’ may alter its value [- 
Werror=conversion]
   str = number();
Improve this answer

3 Comments

I've confirmed -Wconversion would have saved me in this case. I'm changing accepted answer to yours since I think your answer more closely answers my question.
str = 1000; does not use a constructor. It is an assignment, and it uses the assignment operator that takes a char. Since char is an arithmetic type, any numeric value can be converted to char. Yes, if you set your compiler to a non-conforming mode with something like "treat all warnings as errors" you get errors on valid code.
I edited the answer to remove the detail about the constructor because you are 100% correct sorry for that mistake. About turning the warnings into errors i suggested that just because he asked if a set of flags could have saved him and as i did not find a way to stop the compiler from doing that conversion the only way i found it to "block" compilation in that case was to make it see the warning of implicit conversion as an error if there is a better way of doing so please let me know !

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.