1

I'm trying to figure out the issue with this code:

int main(int argc, char *argv[])
{
    char *s;
    scanf("%s\n", s);

    char *t = malloc(sizeof(*s) + 1);

    strcpy(t, s);
    t[0] = toupper(t[0]);
    printf("s: %s\n", s);
    printf("t: %s\n", t);

    free(t);
    return 0;
}

I was trying to upper case the first alphabet of my input to get for example hi! changed into Hi!.

1
  • 4
    scanf("%s\n",s); is reading into uninitialized pointer, sizeof(*s) is 1. Commented Apr 5, 2022 at 15:52

2 Answers 2

3
  1. You are using scanf on a char* without first allocating memory to it. This may lead to segmentation fault. In fact, the following code :

    #include<stdio.h>
    int main()
    {
        char* str;
        scanf("%s", str);
        printf("%s\n", str);
        return 0;
    }
    

    does give segmentation fault sometimes. Instead you can use a char array instead of a char* if you want to avoid malloc before scanf . You can use fgets instead of scanf to prevent writing out of bounds or you can check with scanf itself like this:

    char s[100];
    if(scanf("%99s",s) != 1) { /* error */ }
    

    Also in your code, you have a \n after %s in the scanf statement, that I have removed(assuming you didn't intend this behaviour) . You can see what that does here: Using "\n" in scanf() in C

  2. Secondly, in allocating memory for t, you are using sizeof(*s) which is just sizeof(char) which is defined to be 1. Instead, you should be using strlen(s)+1 which will allocate one more byte than the length of s which should work fine. Also, checking if t==NULL is also recommended.

  3. Before calling toupper the argument must be type-casted into an unsigned char to avoid the risks of undefined behaviour if the char type is signed on the platform. More on this can be found on this answer: https://stackoverflow.com/a/21805970/18639286 (Though the question was a C++ question, the answer actually quotes C standard specifications)

I have edited the above changes into your code to get the following:

int main (int argc,char* argv[])
{

    char s[100];
    if(scanf("%99s",s)!=1)
    {
       fprintf(stderr, "Invalid or missing input\n");
       // scanf should have returned 1 if it successfully read 1 input
    }
    char *t = malloc(strlen(s) + 1); // to store the letters + '\0' at the end
    if(t==NULL)
    {
       fprintf(stderr, "Memory allocation failed\n");
       exit(1);
    }

    strcpy(t, s);
    t[0] = toupper((unsigned char)t[0]);
    printf("s: %s\n", s);
    printf("t: %s\n", t);

    free(t);
    return 0;

}

Some extra points:

You can also use fgets instead of scanf : fgets(s, 100, stdin); as fgets does bound checking.

Also, fgets reads till end of line/file or till the buffer is full, so it will read past the spaces in input, But scanf will stop at a space. Or as Ted Lyngmo pointed out in the comments, scanf with %s reads a word, while fgets reads a line.

But, even if you intend to read only a word, you can use fgets and then use sscanf to read the first word from the string with which you used fgets. Example:

char buffer[20];
char str[sizeof buffer]=""; //initialize with an empty string
if(fgets(buffer, sizeof buffer, stdin)==NULL)
{
  fprintf(stderr, "Error reading input\n");
  exit(1);
}
if(sscanf(buffer, "%s", str)!=1)
{
  // if str was not read
  fprintf(stderr, "String not read\n");
  exit(1);
}
// DO stuff with str 

Here, you don't need to use %19s because we know the size of buffer.

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

4 Comments

@chqrlie added the unsigned char conversion for toupper and initialized str to empty string now.
OK. scanf() is a pain, but getting rid of it is not simple :)
Yes, Checking the return value of sscanf instead of using strcmp after the sscanf seems better. But as I keep modifying the code for using fgets for reading a word, I feel like using scanf with the width specifier seems so much simpler.
I think they both have their uses and using them in combination like you do with fgets + sscanf is often the most secure/less error prone option.
1

As a sidenote to the already great answer, I must recommend GNU Debugger (shortly gdb).

Run "gdb [program name]", the program should be compiled with the -g flag. In the interactive environment, you can run that program by typing r which may yield some useful information. Typing "b <number>" to specify a break in the runtime, then type "s [number]" a number of lines ahead in the program.

There are plenty of ways with gdb to extract information about the running program, I'd heavily recommend reading the docs on it.

Comments

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.