0

I'm maintaining some ugly legacy code with the following function and I am getting

warning: value computed is not used

for the lines marked by comments below:

void ReadKeyValuePipe(char* buffer, char* key, char* value) {
    char* pos;
    char key_str[1024];
    char* val = value;
    sprintf(key_str,"%s:",key);
    if((pos = strstr(buffer,key))) {
        pos += strlen(key_str);
        while (*pos && *pos != '|') {
            *val = *pos;
            *val++; // this is actually used
            *pos++; // so is this
        }
        *val = 0;
    }
}

When I remove those lines, the code breaks. Which makes sense because they appear to be incremented markers.

How do I get the compiler to recognize that these calculations are actually used?

Chris Redford
  • 16,982
  • 21
  • 89
  • 109
  • 1
    They're not used - see the linked duplicate. – BartoszKP Oct 28 '14 at 18:28
  • I'd be more concerned with the latent buffer overflow from calling `sprintf`. At the very least, throw in `assert(strlen(key) + 2 <= sizeof(key_str));` – Casey Oct 28 '14 at 21:21
  • @Casey This entire function seems like the wrong way to do this to me but I have to pick my battles. – Chris Redford Oct 28 '14 at 21:22
  • WTF? `key_str` is only used is an inefficient way to calculate `1 + strlen(key)`? Should that be `strstr(buffer, key_str)` instead? This function truly is horrible. – Casey Oct 28 '14 at 21:24
  • Seriously. If I were going to rewrite this function, I would just convert any `char*` to `string` and do it with [string splitting](http://stackoverflow.com/a/236803/130427). – Chris Redford Oct 28 '14 at 21:26

2 Answers2

4

You're dereferencing val and pos and then incrementing them, but you never use the result of the dereferencing. You can just replace those lines with:

*val = *pos;
++val;
++pos;

Or, probably what the code originally was before somebody "fixed" it:

*val++ = *pos++;
Barry
  • 286,269
  • 29
  • 621
  • 977
1

You can use the -Wno-unused-value compiler option if you are sure that your code is correct. Which is not, since you obviously don't know what *val++; does.

*val++; 
*pos++; 

is the same as:

 val++;
 pos++;
Igor Pejic
  • 3,658
  • 1
  • 14
  • 32
  • 1
    The problem with this approach is that this ugly legacy code and my new additions sometimes do have actual unused values and I rely on the warnings to recognize and eliminate this. (I wasn't the person who down voted you). – Chris Redford Oct 28 '14 at 18:25
  • You can apply `-Wno-unused-value` to only the few lines of code with the problem using the `pragma GCC diagnostic ignored` approach – Walter Oct 28 '14 at 18:43