C++

Forum

 
Memory Error???

klxp9 (10)
Hi,

I have a char array declared as such:

char statement[10000]

First off, is this too large? I have a large sqlite statement that I strcpy into this, and it's pretty big. Anyways, when I run my code through the first time and copy the sql statement into it, it works fine.

When I run it through the second time, though, and I look at it in gdb, it has this in it at the beginning:

"P \017\b8 \017\b \024...|"

After the "|" character, then my sql statement starts. Why is that at the beginning? I did this after the first run:

memset(statement, '\0', sizeof(statement));

...but the error still shows up, so I get an sql syntax error. First off, why is this there at the beginning after a memset? Second, why doesn't another strcpy fill the char with the sql statement from the beginning? Third, how do I get rid of this? Thank you.
helios (17575)
10000 is moderately large. It's not a bad idea to move it to the heap:
char *statement=new char[10000];

Are you saying that this is what's happening?
1
2
3
 
char statement[10000];
memset(statement, '\0', sizeof(statement));
//statement=="P \017\b8 \017\b \024...|"...


Post some code.
klxp9 (10)
This is what I have in main.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
 
int main()
{
   int eight_set_begin[] = {7, 8, 1, 2, 4, 4, 4, 5, 5, 5, 5, 5, 5, 7, 8, 8, 8, 10, 11, 13, /* Eight Hour Set */
			    13, 13, 14, 14, 14, 14, 14, 16, 16, 16, 17, 17, 17, 17, 17, 17, 
			    17, 19, 19, 19, 19, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 
			    20, 20, 20, 20, 21, 22, 22, 23, 23, 23, 23, 23, 23, 23, 23, 23, 
			    23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23};

   int eight_set_end[] =   {8, 9, 2, 3, 4, 4, 5, 5, 5, 5, 5, 5, 6, 8, 8, 8, 9, 11, 11, 13, 13, /* Eight Hour Set */
			    14, 14, 14, 14, 14, 15, 16, 16, 17, 17, 17, 17, 17, 17, 17, 18, 19, 
			    19, 19, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 20, 
			    21, 21, 22, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 23, 
			    23, 23, 23, 23, 23, 23, 23, 24};
   <snip>   

   MusicInfo *start = NULL; /* linked list */
   MusicInfo *current = NULL;
   int option = 0;
   char statement[10000], hd[] = "A";

   do
   {
      cout << "Select an option : " << endl;
      cout << "     0. Exit program" << endl;
      cout << "     1. Get info from database" << endl;
      cout << "     2. Delete first employee in list" << endl;
      cout << "     3. Delete last employee in list" << endl;
      cout << "     4. Move to next employee" << endl;
      cout << "     5. Move to previous employee" << endl;
      cout << "     6. Display list" << endl;
      //cout << "     2. Search for employee" << endl;
      cout << endl;

      cin >> option;

      switch(option)
      {
         case 1:
            strcpy(statement, create_statement(hd, eight_set_begin, eight_set_end));
            querydb_and_populate(start, statement, TRUE);
            memset(statement, '\0', sizeof(statement));
            break;

            <snip>
      }
   }
   while(option != 0);

   free_data(start);

   return 0;
}


In my sqlite.cpp file I have the following.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
 
#include "sqlite.h"
using namespace std;

/*-------------------------------------------------*/
int querydb_and_populate(MusicInfo *&start, char *statement, bool populate)
{
   sqlite3 *db;
   int rc, rows, cols;
   //int rc = 0, rows = 0, cols = 0;
   char **result;
   char *hard_drive = "A", *tablename = "dj.dbs", *zErrMsg;
   MusicInfo *temp, *temp2;

   rc = sqlite3_open(tablename, &db);

   if(rc) {
      printf("Can't open database: %s", sqlite3_errmsg(db));

      if(sqlite3_close(db) == SQLITE_BUSY)
         printf("Unable to close (unfinalized statements)!");

      exit(EXIT_FAILURE);
   }

   rc = sqlite3_get_table(db, statement, &result, &rows, &cols, &zErrMsg);

   if( rc != SQLITE_OK ) {
      printf("SQL Error: %s\n", sqlite3_errmsg(db));
      exit(EXIT_FAILURE);
   }


   if(rows < 1) {
      printf("No rows returned\n");
      exit(EXIT_FAILURE);
   }

   if(populate == FALSE)
      return 0;

   for(int i = 0; i < cols*rows; i = i+cols) { /* This is where populate begins */
      temp = (MusicInfo *) malloc (sizeof(MusicInfo));

      if(!temp) {
         printf("Memory allocation error in querydb_and_populate");
         exit(EXIT_FAILURE);
      }

      temp->id = atoi(result[cols+i]);
      strcpy(temp->title, result[cols+i+1]);
      strcpy(temp->artist, result[cols+i+2]);
      strcpy(temp->hd, result[cols+i+3]); 
      temp->bank = atoi(result[cols+i+4]);
      temp->bookmark = atoi(result[cols+i+5]);
      strcpy(temp->active, result[cols+i+6]);
      strcpy(temp->digital_input, result[cols+i+7]);
      strcpy(temp->genre, result[cols+i+8]);
      temp->price = atof(result[cols+i+9]);
      temp->year = atoi(result[cols+i+10]);
      strcpy(temp->remixer, result[cols+i+11]);
      temp->label_id = atoi(result[cols+i+12]);
      strcpy(temp->class_type, result[cols+i+13]);
      temp->class_number = atoi(result[cols+i+14]);

      temp->next = NULL;

      if(start == NULL)
         start = temp;
      else {
         temp2 = start;
  
         while(temp2->next != NULL)
            temp2 = temp2->next;

         temp2->next = temp;
      }
   }

   sqlite3_free_table(result);
   sqlite3_close(db);
}

/*-------------------------------------------------*/
char *create_statement(char *hd_begin, int begin[], int end[])
{
   char create[10000];
   int total_in_set;

   for(int i=0; i<80; i++)
      total_in_set++;

   for(int j = 0; j <= total_in_set-1; j++){
      strcat(create, "SELECT * FROM (SELECT * FROM dj_music WHERE hd='");
      strcat(create, hd_begin);

      if(strcmp(hd_begin, "A") == 0)
         strcpy(hd_begin, "B");
      else
         strcpy(hd_begin, "A");

      strcat(create, "' AND (");

      for(int k = begin[j]; k < end[j]+1; k++) {
         strcat(create, "(classnumber=");
         strcat(create, itoa(k));
         strcat(create, ")");

         if(k < end[j])
            strcat(create, " OR ");
      }
      strcat(create, ")");
      strcat(create, " ORDER BY random() LIMIT 1)");

      if(j < total_in_set-1)
         strcat(create, " UNION ALL ");

      if(j == total_in_set-1)
         strcat(create, ";");
   }
   return create;
}


As I said earlier, when I go through the first time, everything is fine. After that, though, there's garbabe (?) in the beginning of char statement before the start of the sql statement. How do I get rid of that beginning stuff?

Also, in the above code in create_statement, in the loop for total_in_set, I was trying to loop through the int array to show how many elements were in the list. I thought it worth mentioning is all, before someone said anything about it. I'm going to change that section of code and just pass the total to the function, I think, because it didn't work anyways. That's why I put a static number for the loop.
Last edited on
helios (17575)
create_statement() is returning a pointer to an array that doesn't exist anymore.
Read this: http://www.cplusplus.com/doc/tutorial/dynamic.html
I would suggest you change the design of create_statement() so that it takes a pointer to the statement, rather than returning a pointer to one.

See if the bug is still there after you fix that.

EDIT: But on second thought, there's probably a buffer overflow in some case after case 1. That doesn't mean you shouldn't fix the mistake I mentioned.
Last edited on
klxp9 (10)
Thanks Helios,

I changed my code to this, and it works.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
 
/* sqlite_test.cpp */
/* Test Purposes */

#include <iostream>
#include "../../ll_ops.h"
#include "../../musicinfo.cpp"
#include "../../sqlite.h"
using namespace std;

int main()
{
   <snip>
   MusicInfo *start = NULL;
   MusicInfo *current = NULL;
   int option = 0;
   int length;
   char hd[] = "A";
   char *statement = (char *) malloc (sizeof(char[10000]));

   do
   {
      <snip>

      cin >> option;

      switch(option)
      
         case 1:
            if(start != NULL)
               free_data(start);
            create_statement(hd, statement, eight_set_begin, eight_set_end);
            length = strlen(statement);
            statement[length] = '\0';
            querydb_and_populate(start, statement, TRUE);
            statement[0] = '\0';
            break;
   }
   while(option != 0);

   free(statement);
   free_data(start);

   return 0;
}


I pass a reference to statement in create_statement now. Setting the beginning and ending of statement to the term. character got it to finally work after that. Gotta change the
size of that statement, though. That size just bothers me is all.
helios (17575)
Why don't you use an std::string?
klxp9 (10)
It's because I got a little tired of C++ and moved more of my code to C style. It's easier for me to understand and work with. I never liked using strings from day one. I have less issues using char strings. Thanks for the help!
Zaita (2770)
+1 to using strings. If your going to use C++, then use C++ types.

And, 10000 isn't that big. But I see no bounds checking in your code. Making it extremely vulnerable to buffer overflow attacks.

Now, you may say "i'm a student" or "it's just a small app but it doesn't matter". And you maybe correct. But it's a VERY bad practice to get into and actually tops the 25 most dangerous programming mistakes ("Invalid input valdiation").

http://www.sans.org/top25errors/

IMO. Do it properly, or don't bother.

Edit: Try entering 'A' for your option and see what happens.
http://www.cplusplus.com/forum/articles/6046/
Last edited on
Topic archived. No new replies allowed.