Loop ending condition is not working - C

Question

I have a homework regarding dynamic arrays, therefore I was trying to understand how it works with simple programs.

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


int main()
{
    int cnt,i=0;
    char temp[1001];
    char *obj[5];

    scanf("%d",cnt);

    while(i<cnt){

        scanf("%s",temp);
        obj[i]=malloc(sizeof(char)*(strlen(temp)+1));
        obj[i]=temp;
        printf("%s\n",obj[i]);
        printf("%d\n",i);
        i++;
    }

    return 0;
}

When i get the "cnt" to be equal to 5, by reading from stdin, the program is running forever, though ending condition meets. But when i get the "cnt" to be equal to 5, by assigning it, at the very beginning of the program (not by using scanf) the program works just fine. What might be the reason for this?

Why don't you read the documentation of the function you're using? Why don't you study general concepts in C (like the fact that function arguments are pass-by-value only)? -1 for no effort and a bad, duplicate question. — , Apr 18 '13 at 11:19

score 7 · Answer 1 · edited May 23 '17 at 12:12

7

This:

scanf("%d",cnt);

should be:

/* Always check return value of scanf(),
   which returns the number of assignments made,
   to ensure the variables have been assigned a value. */
if (scanf("%d",&cnt) == 1)
{
}

as scanf() requires the address of cnt.

Also:

Don't cast result of malloc().
sizeof(char) is guaranteed to be 1 so can be omitted from the space calculation in malloc().
Check result of malloc() to ensure memory was allocated.
free() whatever was malloc()d.
Prevent buffer overrun with scanf("%s") by specifying the maximum number of characters to read, which must be one less than the target buffer to allow a space for the terminating null character. In your case scanf("%1000s", temp).
There is no protection for out of bounds access on the array obj. The while loop's terminating condition is i<cnt but if cnt > 5 the an out of bounds access will occur, causing undefined behaviour.

This assigns the address of temp to obj[i]:

obj[i]=temp;

it does not copy (and causes a memory leak). Use strcpy() instead:

obj[i] = malloc(strlen(temp) +1 );
if (obj[i])
{
    strcpy(obj[i], temp);
}

edited May 23 '17 at 12:12

Community

1
1

answered Apr 18 '13 at 11:14

hmjd

120,187
20
207
252

2

+1 especially for checking the return value, this is way too rare. – unwind Apr 18 '13 at 11:17
sizeof(char) is highly likely to be 1, but it is not a guarantee. – mah Apr 18 '13 at 11:19
@mah, it is guaranteed by the standard. – hmjd Apr 18 '13 at 11:21
@mah **it is guaranteed to be one.** – Apr 18 '13 at 11:21
On a 16 bit DSP it's one? Yes, there are processors whose `char` is 16 bits. – mah Apr 18 '13 at 11:21
@mah, yes, the `sizeof` operator is defined with respect to `char`, regardless of the number of bits in `char`. Thus `sizeof(char)` is always `1`. – Jens Gustedt Apr 18 '13 at 12:09

MOHAMED · Answer 2 · 2013-04-18T11:24:59.647

1

you should use this

scanf("%d",&cnt);

BTW:

scanf("%s",temp);

is used in a while loop to read your strings. you have to add space at the beginning of the format specifier to avoid the newline problems. it should be " %s"

edited Apr 18 '13 at 11:24

answered Apr 18 '13 at 11:15

MOHAMED

41,599
58
163
268

score 0 · Answer 3 · answered Apr 18 '13 at 11:17

0

Undefined behavior. You need to pass the address of the variable to scanf():

scanf("%d", &cnt);

But you better not use scanf() anyway. fgets() is simpler and safer to use.

answered Apr 18 '13 at 11:17

What is your objection to using `scanf`, here? – autistic May 07 '13 at 17:53
@undefinedbehaviour 1. As currenly used, you have little to no control over what happens. 1. OP doesn't seem to be checking the return value of `scanf()`, so if it fails, the failure won't be indicated. 2. `scanf()` won't scan entire strings with spaces in them without advanced hacking on character groups, `fgets()` simply returns an entire line which will be available **whenever** it is needed. 3. For that very same reason, using `fgets()` is flexible, since you get a line you can parse as you would like to, 4. `fgets()` explicitly requires a buffer size to be provided -> fail safe. – May 07 '13 at 17:57
@undefinedbehaviour (You can do that with `scanf()` too, but *no beginner on Earth ever* will write `scanf("%*s", sizeof(buf), buf);`) – May 07 '13 at 17:58
You seem to have many misunderstandings regarding `scanf`. When I ask what your objection to using `scanf` is, I mean *using `scanf` correctly*. It is possible to misuse *virtually any* feature of C, and the results are hideous and/or disastrous. That includes `fgets`. `scanf` doesn't *scan* anything, let alone *strings*. It reads input, which isn't in the form of *strings* but characters (there is a difference) and translates the characters to the corresponding types. `fgets` doesn't perform this translation, – autistic May 07 '13 at 18:13
... and in order to perform this translation, extra processing (eg. in the form of `sscanf`) is required. Would you also suggest ignoring the return value of `sscanf`? That could result in undefined behaviour. Why not just use `scanf` (correctly) to begin with? Do note that `scanf`s `%*s` doesn't seem to cause the same functionality that you believe it does. – autistic May 07 '13 at 18:14
You might also want to check the validity of "*`fgets()` simply returns an entire line which will be available whenever it is needed.*" I can think of two circumstances where that isn't the case. – autistic May 07 '13 at 18:16

Loop ending condition is not working - C

3 Answers3