0

I am trying to simply get a user input string. The string is saved (scanf) when the person hits enter or the string hits the NULL character. Yet, when I run my program, and type in the string, it continues input until I type \n or \0 (whichever I have in my if statement. Here's my code:

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

#define MSL 30

char intersperse(char);
char widen_stars(char);

int main(int argc, char *argv[])
{
    char *p, *q, *str1[MSL], *str2[MSL];


    if(str1[MSL+1] != '\0'){    
    printf("Please enter a string of maximum 30 characters: ");
    scanf("%s\n", str1[MSL]);


    }
    printf("%s\n", str1[MSL]);

}

This is by no means finished code, i'm just in the process of writing my program when I snagged this annoying bug. The +1 on my MSL is to make sure the NULL character is read so my string doesn't yell at me, idk if it's necessary, but it's a precaution. Thank you answerers.

Tcranmer
  • 29
  • 1
  • 10
  • The type of `str1` is wrong. It's array of pointers. `MSL+1` is out of bounds. – Eugene Sh. Dec 04 '17 at 22:43
  • Even if str1 was declared correctly (`char *p, *q, str1[MSL], str2[MSL];`) and the if statement tested the last character in the string (`if(str1[MSL-1] != '\0')`) it would never be true because when you create local character array variables they aren't automatically filled with NULL. And if they were why would you need to test it? – Jerry Jeremiah Dec 04 '17 at 22:51
  • I need to be able to exit the input of the string when I press enter, and when I try putting `'\n'` in my if statement it gives me a pointer to integer comparison error. – Tcranmer Dec 04 '17 at 22:56
  • _"The +1 on my MSL is to make sure the NULL character is read"_ the null is not read, it is stored after what is read by `scanf`, but it's not part of what is read. – sidyll Dec 04 '17 at 23:02
  • 2
    "The string is saved (scanf) when the person hits enter or the string hits the NULL character. " + `scanf("%s\n", str1[MSL]);` --> No, user input continues until some non-white-space is followed by white-space. If the user did enter a _null character_, it is treated like a non-white-space. Yet that is not likely the concern here. – chux - Reinstate Monica Dec 04 '17 at 23:10
  • 2
    @chux also, since `\n` will indicate scanf to eat whitespace, it will wait for yet another non-whitespace – sidyll Dec 04 '17 at 23:12
  • @sidyll Agree - that `"\n"` in `scanf("%s\n", str1[MSL]);` is pesky. – chux - Reinstate Monica Dec 04 '17 at 23:13

1 Answers1

3

There are various problems with your code, not just a simple bug. I'll try to describe some of them here.

Turn on compiler warnings. For example, if your array is size 30, there is no element at index 30 (indexes would go from 0 to 29). Your compiler could have warned you about that and other problems.

Store a string in a char array or pointer, but not in a char pointers array. When you wrote:

char *str1[MSL]

You are creating an array of pointers to char, or an array of strings. However, since you're dealing with pointers to char as strings, you'd need to allocate space for them yourself. That's another concept. You probably meant to write an array of char like this:

char str1[MSL]
char *p, *q, str1[MSL], str2[MSL]; /* in your declaration */

Enter at maximum 29 characters. If your string holds 30 char, and you need 1 to mark the null at the end, you've got 29 remaining usable chars. Or… change MSL to 31.

Consider dropping the \n from your scanf format. It will make it read all whitespace (which is what \n represents in a format) waiting for the next non-whitespace. Thus, the input somestring<enter> for example won't be sent directly to your program until the next non-whitespace or EOF.

Give scanf an address. That could be just the array name (which translates to the first element address) or a pointer if you were working with them.

scanf("%s", str1);
scanf("%s", strptr); /* strptr is a char pointer pointing
                          to previously allocated space */

You can limit the length of what scanf reads. To be safe:

scanf("%29s", str1);

Don't use uninitialised data in comparisons. When you wrote if(str1[MSL+1] != '\0'), what did you expect str1 to contain if you had never stored anything yet?


int
main(int argc, char **argv)
{
    char *p, *q, str1[31], str2[31]; /* a lot of unused variables */

    printf("Please enter a string of maximum 30 characters: ");
    scanf("%30s", str1);

    printf("%s\n", str1);

    return 0;
}
sidyll
  • 57,726
  • 14
  • 108
  • 151
  • Thank you for the answer. The unused variables are going to be so I can point my string to them, because I have to be able to overlay the 2 strings where it prints `str1[0] -> str2[0] -> str1[1]` etc.. I fixed the issue and now I'm on the right track, thanks to your help. – Tcranmer Dec 04 '17 at 23:10
  • `scanf("%30s\n", str1);` is a problem in that it will not return until the _next_ word is begun. Suggest to drop the `"\n"` or use `fgets()`. – chux - Reinstate Monica Dec 04 '17 at 23:12
  • @chux just noticed (and commented) about that. Editing. – sidyll Dec 04 '17 at 23:13
  • Agree about turning up compiler warnings, but I doubt that warnings for attempted out-of-bounds array access would be seen. – ad absurdum Dec 04 '17 at 23:30
  • 1
    @DavidBowling well, clang does have an out of bounds attempt warning on by default (I mean, no need to enable anything at all). Not sure about GCC and others though – sidyll Dec 04 '17 at 23:45
  • 1
    It's been a while since I have used clang; I do remember liking the warnings. In the OP code, seems that it would be simple enough for a compiler to catch, but in the more general case of arrays indexed by a variable, the compiler would have to follow all possible code paths to find out-of-bounds attempts. I don't think that even clang does this, but I could be wrong. gcc does have `-Warray-bounds`, enabled by `-Wall`, which warns about subscripts that are _always_ out of bounds (although I can't make this fire in gcc 4.9.4 with some simple test code). – ad absurdum Dec 05 '17 at 00:09