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!.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
Said laasri
  • 19
  • 1

2 Answers2

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.

Prashanth Sriram S
  • 58
  • 5
  • @chqrlie added the ```unsigned char``` conversion for ```toupper``` and initialized ```str``` to empty string now. – Prashanth Sriram S Apr 05 '22 at 18:02
  • OK. `scanf()` is a pain, but getting rid of it is not simple :) – chqrlie Apr 05 '22 at 18:44
  • 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. – Prashanth Sriram S Apr 06 '22 at 02:47
  • 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. – Ted Lyngmo Apr 06 '22 at 16:03
0

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.