CODEWITHSUNDEEP
cvariables
Philipp Hofer
Philipp Hofer

Reputation: 3

Non-used variable impacts program

i have to write a small c program for university, which should print the given parameters again on the command line. The task is to do it with pointers. So my problem is, the program works totally right when the unused variable "bin" is part of the code. But if i try to delete the code or comment it out, the program isn't working anymore (it crashes or doesn't print anything)... But the variable is not used in the program at all. Also i am also able to change the name of it but not its declarated value. Maybe someone can help/ explain what the problem is :) Thank you!

Code: 

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main (int argc, char *argv[])
{
    char *text;
    char bin = '\0'; //<---- unused variable
    char space = 32;
    int i = 0;

    if(argc == 1){
        printf("Bitte Parameter mit angeben!");
        return 1;
    }

    text = malloc(sizeof(char));

    for(i = 1; i < argc; i++){
        text = realloc(text, sizeof(char) * (strlen(argv[i]) + 2 + 
               strlen(text)));
        if(text == NULL)
            return 1;
        text = strcat(text, argv[i]);
        text = strcat(text, &space);
    }

    text++;
    printf("%s", text);
    free(text);
    return 0;
 }

Upvotes: 0

Views: 77

Answers (2)

Zrax
Zrax

Reputation: 1670

text = strcat(text, &space);

This line is taking the address of a single character variable, and then passing it to strcat which expects a nul-terminated string argument. The fact that it works with the bin variable probably means that your compiler has chosen to place the variables next to each other on the stack in such a way that it treats it as the nul-terminator; however, relying on this is certainly undefined behavior.

It would be better to actually provide a string literal or constant, e.g.:

const char space[] = " ";
...
text = strcat(text, space);

or simply:

text = strcat(text, " ");

Note that while this may resolve your immediate problem, there are other issues in the code as well that others have pointed out. Specifically, using text in strlen before initializing the allocated memory and freeing text after the pointer has been moved are both undefined behavior, and I expect you are again just lucky that it happens to work for now.

Upvotes: 1

Clifford
Clifford

Reputation: 93556

Your code concatenates to text without initialising text. You need to start with text as an empty string:

text = malloc(sizeof(char));
text[0] = `\0` ; // empty string

Then passing &space to strcat() is erroneous since space is not a nul terminated string. Simply replace with:

text = strcat(text, " " ) ;

Then later remove the text++ otherwise it will omit the first character, and you cannot free() the modified pointer without causing a heap error:

// text++;   <<< NOT NEEDED and makes free(text) bound to fail.
printf("%s", text);
free( text ) ;

Why it worked with bin present is because you got lucky and it happened to form the nul terminator to &space due to adjacency on the stack. You got lucky again with whatever text pointed to without initialisation, and free'ing the modified pointer may not have caused an error or fault because the program ends without attempting to allocate any further memory.

Upvotes: 3

Related Questions