0

I'm working on a question for a course at my school. I can't seem to use scanf() for some reason, and I've got to use gets().

The question is as follows:

Write a C function stringncpy() that copies not more than n characters (characters that follow a null character are not copied) from the array pointed to by s2 to the array pointed to by s1.

If the array pointed to by s2 is a string shorter than n characters, null characters are appended to the copy in the array pointed to by s1, until n characters in all have been written.

The stringncpy() returns the value of s1.

The function prototype:

char *stringncpy(char * s1, char * s2, int n);

In addition, write a C program to test the stringncpy function. Your program should read the string and the target n characters from the user and then call the function with the user input.

In this program, you are not allowed to use any functions from the C standard String library

When I run the program after a successful build, I keep getting the following error.

Enter the string:
this is atest
Enter the number of characters:
stringncpy(): ╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠this

The code for the implementation is as follows:

#include <stdio.h> 
char *stringncpy(char *s1, char *s2, int n);
int main()
{
    char sourceStr[40], targetStr[40], *target;
    int length;
    printf("Enter the string: \n");
    //scanf("%s",sourceStr); //doesn't work for some reason
    gets(sourceStr); 
    printf("Enter the number of characters: \n");
    scanf("%d", &length);
    target = stringncpy(targetStr, sourceStr, length);
    printf("stringncpy(): %s\n", target);
    return 0;
}
char *stringncpy(char *s1, char *s2, int n)
{
    /* Copy source to target */
    int i, j;
    for (i = 0; i < n; i++)
    {
        if (s2[i] == '\0')
        {
            break;
        }
        else
        {
            s1[i] = s2[i];
        }
    }
    //s1[i + 1] = '\0';
    for (j = i + 1; j <= n; j++)
    {
        s1[j] = '\0';
    }
    return s1;
}

Can anyone please tell me what's wrong with this? Thank you.

Donald Duck
  • 8,409
  • 22
  • 75
  • 99
Jerome Papalie
  • 21
  • 2
  • 10
  • 1
    Read *carefully* the documentation of [scanf](http://en.cppreference.com/w/c/io/fscanf) and of [fgets](http://en.cppreference.com/w/c/io/fgets). Some systems (e.g. POSIX) have [getline](http://man7.org/linux/man-pages/man3/getline.3.html). Compile your code with all warnings and debug info (`gcc -Wall -g` with [GCC](http://gcc.gnu.org/)...). **Use the debugger** `gdb` and [valgrind](http://valgrind.org/). Your fix-my-code question (or do-my-homework one) is **off-topic**. – Basile Starynkevitch Apr 08 '17 at 05:29
  • **Don't use `gets`** : it is dangerous and obsolete. – Basile Starynkevitch Apr 08 '17 at 05:30
  • Note that [`gets()` is to dangerous to be used — ever!](http://stackoverflow.com/questions/1694036/why-is-the-gets-function-dangerous-why-should-it-not-be-used) – Jonathan Leffler Apr 08 '17 at 05:35
  • What keys did you type before the first `t` and after the last `t` in `"this is atest"`? – chux - Reinstate Monica Apr 08 '17 at 06:31

3 Answers3

1

You shouldn't use gets; it's deprecated. Use fgets instead.

The following loop seems to violate the following requirement:

If the array pointed to by s2 is a string shorter than n characters, null characters are appended to the copy in the array pointed to by s1, until n characters in all have been written.

for (j = i + 1; j <= n; j++)
{
    s1[j] = '\0';
}

This is causing n+1 characters to be written. Is it possible that this is causing an off-by-one (buffer overflow), which is undefined behaviour, and likely to cause illogical behaviour?

It's also important to note that scanf returns a value, which if you were to validate you'd have spotted an error early on!

That error is, scanf("%s", ...) is not analogous to gets(...). gets reads a line of input, while it's more appropriate to think of scanf("%s", ...) as though it reads a word (whitespace delimitered token) of input.

Logic follows that if your input is "Hello world\n" followed by "13\n", gets would read "Hello world\n" into sourceStr, then "13\n" would be left in stdin to be assigned to length by scanf later.

On the other hand, if you use scanf("%s", ...) then "Hello" will be read into sourceStr, and "world\n" "13\n" will be left in stdin to be... wait, how do you convert "world\n" to a decimal integer? Is this also likely to cause illogical behaviour?

Check your return value! This is important for fgets, too. Since both of your calls to scanf expect one field, they should both return 1 when successful. e.g.

int x = scanf("%s", sourceStr);
if (x != 1) { /* XXX: something went wrong! */ }

x = scanf("%d", &length);
if (x != 1) { /* XXX: something went wrong! */ }

If you really must use scanf to read a line, the appropriate conversion specification would be %[^\n], which like %s has similar issues to gets (namely, it doesn't know the size of the target array, so it might cause overflows). You should prefix the specifier with a length, which involves using magic numbers (e.g. %39[^\n] in your case). Better to just use the best tool for the job, which in this case is fgets.

Community
  • 1
  • 1
autistic
  • 1
  • 3
  • 35
  • 80
  • I would suggest that [getline(3)](http://man7.org/linux/man-pages/man3/getline.3.html) is better than `fgets`. But only defined in POSIX – Basile Starynkevitch Apr 08 '17 at 05:42
  • 1
    @BasileStarynkevitch I consider the desirable qualities represented by the word "better" to be "able to use on systems outside of POSIX" and "able to use with automatic storage duration, as opposed to "locking us into allocated storage duration". `argv` is likely more suitable yet, as this wouldn't place an explicit limit in size, fulfilling the only desirable quality of `getline`. Nonetheless, thanks for your two cents. That was a very valuable pedantic note. – autistic Apr 08 '17 at 05:48
0

From looking at your inputs,

Enter the string: this is atest Enter the number of characters: stringncpy(): ╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠╠this

it is clear that it didn't give you the chance to enter a number. This means that, as above answer indicated, the characters after the first white space stayed in the stdin buffer and it was automatically assigned to the variable 'length'.

So you want to check out the value that is assigned to variable "length".

In this case string was assigned to an int variable and we got this weird results.

Here is another way of getting an integer value from a user prompt:

printf("Enter the number of characters: \n");
fgets( sLength, 10, stdin );
length = atoi(sLength);

When you use scanf(), the problem with this approach is that the Carriage Return character stays in STDIN buffer.

Nguai al
  • 958
  • 5
  • 15
-1

scanf("%s",sourceStr); doesn't work with the strings which has spaces or newline characters in them. As you have given the input "This is a text."

you have to make use of scanset or gets function

scanf("%[^\n]s",sourceStr);

this will read the whole string along with spaces until a newline character is encountered.