clang complaint about string comparison


  • Moderators

    I am still getting used to clang model in new creator version

    Today I stumbled over where str is std::string.

    //    if ( str.substr( 0, 9 ) == "Name week")
        if ( ! strncmp ( str.c_str(), "Name week", 9 ) )
    

    the commented line got the complaint this is not defined and one should use "strncmp". I am not really a C++ purist and can accept things when working.
    However, that construct was not really going down well and I used google to find a bit of explanation. std::string has even its own compare function since c++98 standard.

    Personally I try to get away from pure C and tend to go for C++.

    Therefore a couple of questions thrown to the experts:
    Why the recommendation is to strncmp and not string::compare?
    Is this because std::string is standard template and not Qt?
    Also does one know why the direct comparison is not a good choice? At first sight possible ambiguities are not obvious.

    Also the clang modeler is a mighty tool, I am certainly impressed. However, when changing to strncmp it did not tell that the appropriate include is missing. ;)


  • Moderators

    I can't answer all the questions you have but I can say you should definitely be using string::compare if you using std::string. I look at the strncmp as incorrect. It's odd that the clang modeler is viewing it as wrong. I would say that this is just a bug in the modeler more than a reason to switch.

    Personally I try to get away from pure C and tend to go for C++.

    If you are being a C++ purist you don't want to use C string compare functions for sure. Now-a-days People that learn C++ from scratch probably don't even know the old C functions since STL has all that for them now. C and C++ aren't as mixed as they used to be with the C++98/11/17/etc standards.

    Also does one know why the direct comparison is not a good choice?

    I think it is the good/proper choice. :)


  • Qt Champions 2017

    hi all,

    just a guess: the second parameter is a string constant, and it may be cheaper to convert with c_str() (effective a pointer) than converting the constant to std::string.

    that all is given there is no const char * overload for compare (which I haven't checked).

    Edit: or it is rather the substr() which is not cheap. does it allocate?

    Edit2: try compare() as @ambershark suggested. it shoul work best.



  • @koahnig what version of QtCreator are we talking about, the 4.7rc ?
    Because with 4.6.2 , this:

    std::string str("Name week blablabla");
        if ( str.substr( 0, 9 ) == "Name week")
            qDebug() << "is substr";
    

    does not create a warning for me.


  • Qt Champions 2017

    @J.Hilk Do you have Clang Code Model enabled?


  • Moderators

    @J.Hilk said in clang complaint about string comparison:

    @koahnig what version of QtCreator are we talking about, the 4.7rc ?
    Because with 4.6.2 , this:

    std::string str("Name week blablabla");
        if ( str.substr( 0, 9 ) == "Name week")
            qDebug() << "is substr";
    

    does not create a warning for me.

    I have already upgraded to 4.7.0 as soon as it came out. I doubt that you see it with 4.6.2



  • @aha_1980
    oh, you're right, Iassumed it was a new default feature, but it's on all my PCs disabled because the corresponding plugin is not loaded

    Well, I'm out of the discussion than :(



  • I didn't realize, that 4.7 was released, its still in my Preview in the maintenance repo, but I was able to update on my macos device.

    So, under what diagnostic configuration das this accure? For me, it's by default set to Clang-only checks for almost everything [build-in] and I don't have a warning with the above mentioned code.


  • Moderators

    @J.Hilk

    That is basically the set I am using as well. The only difference that I added a setting for avoiding the qDebug() warning issue in release mode encountered with Qt 5.4.

    Some times you have to wait a moment for warnings to show.


  • Moderators

    Basically I was a bit thrown off by the recommendation to use old c-style while in other cases I am receiving warnings when I am using c-style casting. The discussion has been started to back up by bearings. Sometimes one (I) tend to stick to something ineffective.

    std::string:compare has even in one version the requested substr part integrated.

    Thanks to all for reponses


Log in to reply
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.