CODEWITHSUNDEEP
cwhile-loopstructure
Ahmed Elsenbawy
Ahmed Elsenbawy

Reputation: 19

How to fix this while loop used to fill array of struct inside it another array of struct

I'm filling an array of struct within a code and inside it another array of the struct but I get stuck in the loop. I tested every branch and loop: it works. But the loop doesn't work.

In service_data _func I try to analyze text and add it to the struct.

I call it in the main function and pass text to it and try to print a value stored I use the print command in each step in the loop it works but getting out of it it doesn't work.

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

struct service_charge {
    int from;
    int to;
    int charge;
    int slap;
    char percentage[5];
};
struct service {
    int id;
    int provider_id;
    char name[30];
    int price_type;
    int min_value;
    int max_value;
    int sort_order;
    char inquiry_required[5];
    struct service_charge charge_arr[10];
};
struct service serv_data[8];
char text[1000]="{\"success\": true,\"language\": \"en\",\"action\":
                \"GetServiceList\",\"version\": 1,\"data\": {\"service_list\":
                [{\"id\": 4806,\"provider_id\": 581,\"name\": \"Bill Payment (MG SC
                AC)\",\"price_type\": 0,\"min_value\": 30,\"max_value\":
                10000,\"sort_order\": 2,\"inquiry_required\":
                true,\"service_charge_list\": [{\"from\": 1,\"to\": 547,\"charge\":
                1,\"slap\": 1,\"percentage\": true1},{\"from\": 2,\"to\":
                54875,\"charge\": 4,\"slap\": 5,\"percentage\": true1},,
                {\"from\": 2,\"to\": 68945,\"charge\": 4,\"slap\":
                5,\"percentage\": true2}]}";

void service_data_func (char text[]) {

    int i=0;
    int Wstart=0;
    int Wend=0;
    char name[19]= {0x20};
    char name1[19]= {0x20};
    int menunum=0;
    int len;
    len=strlen(text);
    int menunum_charge=0;

    while (1)//while ALL
    {
        if(i>=len) {
            break;
        }
        if(text[i] == '"' && text[i+1] == 'i'&&
                text[i+2] == 'd') {
            while (1) { //while "id

                if(text[i] == ':') {
                    Wstart=i+1;
                    Wend=0;
                    i++;
                } else if(text[i] == ',' || text[i] == '}' ) {

                    Wend=i;

                    strncpy(name,text+Wstart,Wend-Wstart);
                    serv_data[menunum].id=atoi(name);
                    memset(name, 0, sizeof(name));
                    i++;
                    break;
                } else {
                    i=i+1;
                }

            }//while id
        } else if(text[i] == 's' && text[i+1] == 'e'&&
                  text[i+2] == 'r'&& text[i+3] == 'v'&& text[i+4] == 'i'&&
                  text[i+5] == 'c'&& text[i+6] == 'e'&& text[i+7] == '_'&& text[i+8]
                  == 'c'&& text[i+9] == 'h'&& text[i+10] == 'a'&& text[i+11] ==
                  'r'&& text[i+12] == 'g'&& text[i+13] == 'e'&& text[i+14] == '_'&&
                  text[i+15] == 'l'&& text[i+16] == 'i'&& text[i+17] == 's'&&
                  text[i+18] == 't') {
            while (1)//while ALL
            {
                if(i>=len) {
                    break;
                }
                if(text[i] == 'f' && text[i+1] == 'r'&&
                        text[i+2] == 'o'&& text[i+3] == 'm') {
                    while (1) { //while from

                        if(text[i] == ':') {
                            Wstart=i+1;
                            Wend=0;
                            i++;
                        } else if(text[i] == ',' || text[i] == '}' ) {

                            Wend=i;

                            strncpy(name,text+Wstart,Wend-Wstart);

                            serv_data[menunum].charge_arr[menunum_charge].from=atoi(name);
                            memset(name, 0, sizeof(name));
                            i++;
                            break;
                        } else {
                            i=i+1;
                        }

                    }
                } else {
                    i++;
                }

            }
        } else {
            i++;
        }

    }

}

int main()
{
    service_data_func(text);
    printf("%d\n",serv_data[0].charge_arr[0].from);


    return 0;
}

Upvotes: 0

Views: 146

Answers (1)

bruno
bruno

Reputation: 32596

When you reach the case "service_charge_list" and you extracted the value of "from" you missed to add a break; to go out of the while (1)//while ALL so you continue to search and you reach the next "from" even that one doesn't correspond to the same case, so you replace the value 1 by 2.

You can for example replace :

            if(text[i] == 'f' && text[i+1] == 'r'&&
                    text[i+2] == 'o'&& text[i+3] == 'm') {
                while (1) { //while from
                ...
                }
            } else {
                i++;
            }

by (the else can be removed)

            if(text[i] == 'f' && text[i+1] == 'r'&&
                    text[i+2] == 'o'&& text[i+3] == 'm') {
                while (1) { //while from
                ...
                }
                break;
            } else {
                i++;
            }

now the execution print 1 and corresponds to serv_data[0].id valuing 4806

Additional remarks :

(1) A code like

if(text[i] == 's' && text[i+1] == 'e'&&
   text[i+2] == 'r'&& text[i+3] == 'v'&& text[i+4] == 'i'&&
   text[i+5] == 'c'&& text[i+6] == 'e'&& text[i+7] == '_'&& text[i+8]
   == 'c'&& text[i+9] == 'h'&& text[i+10] == 'a'&& text[i+11] ==
   'r'&& text[i+12] == 'g'&& text[i+13] == 'e'&& text[i+14] == '_'&&
   text[i+15] == 'l'&& text[i+16] == 'i'&& text[i+17] == 's'&&
   text[i+18] == 't')
 ...

is unreadable, difficult to maintain and can easily contains an error. It can be just that :

if (!strncmp(text + 1, "service_charge_list", 19))
  ...

of course this is also true in the smaller cases.

(2) It is dangerous and useless to give a size in :

char text[1000]="{\"success\": true,\"language\": \"en\",\"action\":\"GetServiceList\",\"version\": 1,\"data\": {\"service_list\":[{\"id\": 4806,\"provider_id\": 581,\"name\": \"Bill Payment (MG SCAC)\",\"price_type\": 0,\"min_value\": 30,\"max_value\":10000,\"sort_order\": 2,\"inquiry_required\":true,\"service_charge_list\": [{\"from\": 1,\"to\": 547,\"charge\":1,\"slap\": 1,\"percentage\": true1},{\"from\": 2,\"to\":54875,\"charge\": 4,\"slap\": 5,\"percentage\": true1},,{\"from\": 2,\"to\": 68945,\"charge\": 4,\"slap\":5,\"percentage\": true2}]}";

just do

char text[]="{\"success\": true,\"language\": \"en\",\"action\":\"GetServiceList\",\"version\": 1,\"data\": {\"service_list\":[{\"id\": 4806,\"provider_id\": 581,\"name\": \"Bill Payment (MG SCAC)\",\"price_type\": 0,\"min_value\": 30,\"max_value\":10000,\"sort_order\": 2,\"inquiry_required\":true,\"service_charge_list\": [{\"from\": 1,\"to\": 547,\"charge\":1,\"slap\": 1,\"percentage\": true1},{\"from\": 2,\"to\":54875,\"charge\": 4,\"slap\": 5,\"percentage\": true1},,{\"from\": 2,\"to\": 68945,\"charge\": 4,\"slap\":5,\"percentage\": true2}]}";

or

const char * text="{\"success\": true,\"language\": \"en\",\"action\":\"GetServiceList\",\"version\": 1,\"data\": {\"service_list\":[{\"id\": 4806,\"provider_id\": 581,\"name\": \"Bill Payment (MG SCAC)\",\"price_type\": 0,\"min_value\": 30,\"max_value\":10000,\"sort_order\": 2,\"inquiry_required\":true,\"service_charge_list\": [{\"from\": 1,\"to\": 547,\"charge\":1,\"slap\": 1,\"percentage\": true1},{\"from\": 2,\"to\":54875,\"charge\": 4,\"slap\": 5,\"percentage\": true1},,{\"from\": 2,\"to\": 68945,\"charge\": 4,\"slap\":5,\"percentage\": true2}]}";

(3) In

char name[19]= {0x20};
char name1[19]= {0x20};

there is no reason to set the first char of name to ' ', and name1 is unused and can be removed

(4) In cases like that :

   if(text[i] == '"' && text[i+1] == 'i'&&
             text[i+2] == 'd') {
         while (1) { //while "id

             if(text[i] == ':') {
               ...

you test again the first 3 characters, better to do i += 3; before the while

(5) You duplicate a similar codes to search for keywords and to extract values, you cannot continue to do like that for all the cases, it is already too much with the few cases you manages, use dedicated functions for that

(6) A kiran Biradar says in a remark you have several cases of infinite loops finally producing accesses out of text with the associated undefined behavior if text is invalid

Upvotes: 3

Related Questions