Why break gets me out of 2 loops at once and how to fix it

Question

Ok, so the idea of the task I have (I am the student) is to allow user to insert a string of words in this form: num1_num2_num3..._numN. The code should create an array X, give it memory dynamically and then I should fill X with numbers from string user inserted. Simple as that. Well, in the function stringuniz() I thought I had it all figured out but it simply wont work. It gets the first number well but it then stops and I think its because of the break. Break behaves (if I am right) like it breaks the entire code and not just the loop. Do you guys have an idea why is this happening?

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

void stringuniz(char *);
int *x;

int main(){

    char s[50];
    int i;

    puts("Unesite string brojeva u formatu br1_br2_...brN: ");
    gets(s);

    stringuniz(s);

    for(i=0;i<(sizeof(x)/sizeof(int));i++)
        printf("%d",x[i]);
}

void stringuniz(char *s){

    int duz,c=0,i,j,k=0,m=0;
    char b[10];

    duz=strlen(s);

    for(i=0;i<duz;i++)
        if(s[i]=='_')
            c++;

    x=(int*)malloc((c+1)*sizeof(int));
    if(x==NULL) exit(1);

    for(i=0;i<c+1;i++){
        for(j=m;j<duz;j++){
            if(s[j]!='_'){
                b[k++]=s[j];
                m++;
            }
            else{
                b[k]='\0';
                x[i]=atoi(b);
                k=0;
                m++;
                break;
            }

        }
    }
}

http://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used — William Pursell, Nov 27 '16 at 20:21
Beacause `sizeof(x)` doesn't return the allocated size (only the size of the pointer), the `sizeof(x)/sizeof(int)` is equal to 1 and only the first number is displayed. Modify your `void stringuniz(char *s)` to `int stringuniz(char *s)` and add `return (c+1);` at the end. — J. Piquard, Nov 27 '16 at 20:38

score 1 · Accepted Answer · answered Nov 27 '16 at 20:40

This

(sizeof(x)/sizeof(int)

won't give you the size of the array. sizeof(x) is the bytesize of int* (likely 4 or 8). You'll need to remember the size as implied by the number of _ in the string.

Also you have some off-by-one errors in there and for future reference, you might want to choose more descriptive variable names for code you decide to post publicly.

The code worked for me once I changed it to:

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

void stringuniz(char *);
int *x;
int x_size = 0;

int main(){
    char s[50];
    int i;
    puts("Unesite string brojeva u formatu br1_br2_...brN: ");
    fgets(s,50,stdin);
    stringuniz(s);
    for(i=0;i<x_size;i++)
        printf("%d\n",x[i]);
}
void stringuniz(char *s){
    int duz,c=0,i,j,k=0,m=0;
    char b[10];
    duz=strlen(s);
    for(i=0;i<duz;i++)
        if(s[i]=='_')
            c++;
    x=malloc((c+1)*sizeof(int));
    x_size = c+1;
    if(x==NULL) exit(1);
    for(i=0;i<=c+1;i++){
        for(j=m;j<=duz;j++){
            if(s[j]!='_' && s[j]!='\0'){
                b[k++]=s[j];
                m++;
            }
            else {
                b[k]='\0';
                x[i]=atoi(b);
                k=0;
                m++;
                break;
            }

        }
    }
}

Bodo Thiesen · Answer 2 · 2016-11-27T20:44:59.557

void stringuniz(char *);
int *x;

int main(){
    [...]
}

void stringuniz(char *s){
    [...]
}

I don't know why many ppl teach it this way, but there is absolute no use in having main somewhere in the middle of a source file, and putting it at the end also allows you to get rid of the forward declarations. So, I would write it this way:

int *x;

void stringuniz(char *s){
    [...]
}

int main(){
    [...]
}

Then you should start using the space character more.

    stringuniz(s);

    for(i=0;i<(sizeof(x)/sizeof(int));i++)
        printf("%d",x[i]);

In a comment, alain already pointed out, that sizeof(x) will return the size of a pointer. So, you need a different way to figure out the size of the array. One way is to add a variable size_t x_len; besides int * x;. Also, you should use curley brackets even for one line statements, believe me, not only makes it the code more readable, it also prevents introducing bugs on later changes.

    for (i = 0; i < x_len; i++) {
        printf("%d", x[i]);
    }

.

void stringuniz(char *s){
    int duz,c=0,i,j,k=0,m=0;
    char b[10];

b will hold the word the user enters. If his word is longer then 9 characters, you get a buffer overflow here.

    duz=strlen(s);

    for(i=0;i<duz;i++)
        if(s[i]=='_')
            c++;

You are counting the number of words here. So, please use more descriptive names like num_words instead of c. BTW: This is the x_len mentioned above.

    x=(int*)malloc((c+1)*sizeof(int));

No need to cast return value of malloc. Actually it might hide bugs. Also, I would use sizeof(*x) instead of sizeof(int), because if you change the type of x, in your statement, you also would have to change the malloc call. In my statement, the malloc call doesn't need to be touched in any way.

    x = malloc((c+1) * sizeof(*x));

    if(x==NULL) exit(1);

    for(i=0;i<c+1;i++){
        for(j=m;j<duz;j++){
            if(s[j]!='_'){
                b[k++]=s[j];

You are constantly overwriting b with the next word being read. Since you're not using it anyway, you can just skip this line.

                m++;
            }
            else{
                b[k]='\0';
                x[i]=atoi(b);
                k=0;
                m++;
                break;

And this break; only breaks out of the innermost for (j-loop.

Why break gets me out of 2 loops at once and how to fix it

2 Answers2