Argument types for the strchr() function

[Tord Romstad]

Bryan is trying to compile a Windows executable of Glaurung CCT8, but is having problems with the following function:

Code: Select all

void push_button(const char *button_name) {
  uci_option_t *u;
  char *c;
  // Remove trailing newline character:
  c = strchr(button_name, '\n');
  if(c != NULL) *c = '\0';

  u = option_by_name(button_name);
  if(u != NULL) strcpy(u->value, "true");
}


Bryan's compiler gives the following error message:

Code: Select all

ucioption.cpp(158) : error C2440: '=' : cannot convert from 'const char *'
to 'char *'
Conversion loses qualifiers


Line 158 is the line with the call to strchr() in the push_button() function. I'm confused by this, because the manual page for strchr() on my machine claims that the first argument is a 'const char *', not a 'char *':

Code: Select all

STRCHR(3)                BSD Library Functions Manual                STRCHR(3)

NAME
     strchr, strrchr -- locate character in string

LIBRARY
     Standard C Library (libc, -lc)

SYNOPSIS
     #include <string.h>

     char *
     strchr(const char *s, int c);

     char *
     strrchr(const char *s, int c);


This problem should be easy to fix: Changing the first argument of my push_button() function from a 'const char *' to a 'char *' should work just fine, and does no harm. But which form is correct, according to the C standard? What is the type of the first argument to strchr() supposed to be?

Tord

[Volker Annuss]

Hello Tord,

removing const from your parameter button_name is correct because it will be modified by *c = '\0'.

Your manual is correct according to the C standard. (I don't have the standard here, but a draft from ISO/IEC from Jan 18, 1999 that is very close to the standard.)

The problem that gives the error is not the argument list to strchr. The error is that Bryan's compiler returns a const char* that cannot get assignet to the non-const char *c.

Greetings,
Volker

[Tord Romstad]

Hello Tord,

removing const from your parameter button_name is correct because it will be modified by *c = '\0'.

Hello Volker,

Indeed you are right. Strange that GCC doesn't catch this error, and that Bryan's compiler reports an error at the wrong line in the code.

I've updated the Glaurung CCT8 source code on my site once again with this little error corrected. Those of you who managed to compile the code previously don't have to download again. This error was in the UCI option handling code, and has nothing to do with actual play. If you were able to compile the old code at all, your binary will work just as well as one compiled from the previous iteration of the source code.

Thanks for your help!

Tord

[Volker Annuss]

Hello Tord,

the solution may be that Bryan uses a C++ Compiler.

In C++ there are two overloaded versions of strchr:

Code: Select all

      char *strchr(       char*, int )
const char *strchr( const char*, int )


Depending on the type of the arguments, the compiler takes the one that fits best. Return types are not used in overloading resolution.

in C there is only

Code: Select all

char *strchr( const char*, int )


Greetings,
Volker

[Tord Romstad]

Hello Tord,

the solution may be that Bryan uses a C++ Compiler.

That's a very good guess, but it cannot be the full explanation. I use a C++ compiler, too. Glaurung 1.0.2 and earlier were plain C, but Glaurung CCT8 is coded in C++.

I didn't know about the two overloaded versions of strchr in C++, by the way. Thanks for the information.

Tord

[Alessandro Scotti]

The:

Code: Select all

char *strchr( const char*, int );


prototype is insane IMO. You can write code that the compiler knows is wrong but is forced to accept, and that will happily crash at runtime:

Code: Select all

    const char * s = "Hello, world\n";
    char * p = strchr( s, '\n' );
    if( p != NULL ) *p = '\0';


This is compiler and platform dependent, but in most cases the compiler will put string constants in read-only memory, which core dumps (i.e. abends, traps, bombs, etc.) when you try to write it.

Basically, this form of strchr is a hack introduced to support the "const" keyword in C: the char * return could not be changed without breaking millions of lines of code, but keeping the char * argument would have made it impossible to use the function with const char * strings.

[Alessandro Scotti]

Re: the push_button() function, I would keep the const char * prototype, as it seems to suit the function better. I don't have Glaurung's code right here but would also expect option_by_name() to take a const char *.
A viable solution could be stripping newline characters before calling push_button, maybe even right after reading input if they are not needed elsewhere in the program.

[Jim Ablett]

Just compiled it with 32bit Intel compiler 9 - no errors, runs ok,
(at least with 1 cpu - that's all I've got!

http://www.ihud.com/file.php?file=28020 ... -intel.zip

Jim.

[Sven Schüle]

Hi Tord,

as Alessandro proposed, consider to split the code like this (C++ compiler required):

Code: Select all

void strip_newline(char *s) {
    if (s != 0) {
        // Remove trailing newline character:
        char *c = strchr(s, '\n'); // will always select the 'non-const' variant
        if (c != 0) *c = '\0';
    }
}

void push_button(const char *button_name) {
  uci_option_t *u = option_by_name(button_name);
  if (u != 0) strcpy(u->value, "true");
}

void some_other_function() {
    char button_name[100];
    get_the_button_name_from_somewhere(button_name);
    strip_newline(button_name); // button_name modified here
    push_button(button_name); // button_name used as 'const' here
}

You will probably need a strip_newline() function more than once.

Using NULL instead of 0 to represent a zero pointer is useless in C++. Just use 0, it is correct and recommended.

Sven

[Alessandro Scotti]

One thing that looks suspicious is the strip newline function... what about multiple characters, i.e. '\r' followed by '\n'?

[Tord Romstad]

Hi Tord,

as Alessandro proposed, consider to split the code like this (C++ compiler required):

(Code snipped)

I agree with both of you that 'const char *' is the best type to use for the argument to push_button(). It is also more consistent with the rest of my code. I'm not sure I'll use exactly the solution you propose, but I'll probably do something rather similar.

I'm not going to release yet another very slightly modified version of the Glaurung CCT8 source code, though. My current code works and compiles correctly, which is the most important thing right now. I will clean up this little detail of the code before I release the next public version.

Using NULL instead of 0 to represent a zero pointer is useless in C++. Just use 0, it is correct and recommended.

I prefer using NULL. Of course NULL always equals 0, but I think the code is often clearer to read when I keep the distinction. NULL, 0 and false all mean the same thing to the compiler, but they don't mean the same thing to me.

Tord

[Sven Schüle]

One thing that looks suspicious is the strip newline function... what about multiple characters, i.e. '\r' followed by '\n'?

Hi Alessandro,

strip_newline() does what its name suggests: it strips a trailing '\n'. This is sufficient when dealing with strings originating from functions like fgets().

If, however, there is also a need to deal with strings possibly containing '\r\n' at the end (normally not produced by fgets()) you might want to write an additional strip_crlf() function; you may guess its implementation . Then, strip_crlf() may also become intelligent enough to detect whether there is a '\r\n', a '\n', or none of them present - if you like and need it.

Sven