0

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;
}
5
  • Try a debugger instead of printfs, it is more powerful and easier Commented Jan 31, 2019 at 9:55
  • 1
    You should write the question properly: use correct English, use punctuations (you never inserted a fullstop between sentences), use capital letters when needed (I instead of i). How do you expect people to understand your question? Also indent your code properly. Commented Jan 31, 2019 at 9:55
  • What if if(text[i] == ',' || text[i] == '}' ) condition is always false? You never break out of the loop. Commented Jan 31, 2019 at 10:08
  • I found why you get the value 2 rather than 1, and more problems, I edited my answer. Commented Jan 31, 2019 at 11:53
  • @kiranBiradar yes that code is very fragile and do not manage a non expected input Commented Jan 31, 2019 at 11:54

1 Answer 1

3

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

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.