Hello I am working on a program that requires me to use structs. I am
allocating memory for the struct and the variables defined in them.
I am sure there is something incorrect with my code because there is
undefined behavior when running the program.
I am posting the code so if any of you can catch the problem.
I am new still to this so I am sure it's something trivial. I appreciate
any help.
Also are there any good guides/books that can get into more detailed
explanation of pointers and memory allocation so that I can have a more
in depth understanding?
Here is the code I am working with:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sqlite3.h>
#define BUFFER_SIZ 128
typedef struct questionPacket
{
char *id;
char *data;
char *choiceA;
char *choiceB;
char *choiceC;
char *choiceD;
}qpacket;
typedef struct answerPacket
{
char answer[1];
}anspacket;
void createQuestionPacket(struct questionPacket **qp);
void createAnswerPacket(struct answerPacket **ap);
void initializeSQLite();
int getQuestionPacket(struct questionPacket **qp);
static int getQuestionCallback(void *fpArg, int argc, char **argv, char
**azColName);
static sqlite3 *db;
static char *dbfile = "millionaire.db";
static int retval;
static char *zErrMsg = 0;
qpacket *callbackArg;
int main()
{
qpacket *qp;
anspacket *ap;
int ret = -1;
/*test*/
createQuestionPacket((struct questionPacket **)&callbackArg);
/* allocate packet memory */
createQuestionPacket((struct questionPacket **)&qp);
createAnswerPacket((struct answerPacket **)&ap);
initializeSQLite();
printf("after initializeSQLite\n");
ret = getQuestionPacket((struct questionPacket **)&qp);
printf("id is %s\n",callbackArg->id); /* id has correct
value */
printf("question is %s\n", callbackArg->data); /* data has incorrect
value */
return 0;
}
void createQuestionPacket(struct questionPacket **qp)
{
if((*qp=(struct questionPacket *)malloc(sizeof(qpacket)))==NULL)
{
fprintf(stderr, "No memory was allocated for change question packet!
\n");
exit(0);
}
(*qp)->id = malloc(sizeof(char) * 8);
(*qp)->data = malloc(sizeof(char) * 256);
(*qp)->choiceA = malloc(sizeof(char) * 128);
(*qp)->choiceB = malloc(sizeof(char) * 128);
(*qp)->choiceA = malloc(sizeof(char) * 128);
(*qp)->choiceA = malloc(sizeof(char) * 128);
}
void createAnswerPacket(struct answerPacket **ap)
{
if((*ap=(struct answerPacket *)malloc(sizeof(anspacket)))==NULL)
{
fprintf(stderr, "No memory was allocated for double dip packet!\n");
exit(0);
}
}
void initializeSQLite()
{
/* connect to DB */
retval = sqlite3_open_v2(dbfile, &db, SQLITE_OPEN_READWRITE, NULL);
if( retval )
{
fprintf(stderr, "Can't open database: %s\n", sqlite3_errmsg(db));
sqlite3_close(db);
exit(1);
}
}
int getQuestionPacket(struct questionPacket **qp)
{
char *questionSqlStmt;
questionSqlStmt = malloc(BUFFER_SIZ);
strcpy(questionSqlStmt,"select
id,question,choice_a,choice_b,choice_c,"\
"choice_d from questionTbl ORDER BY "\
"RANDOM
() LIMIT 1;");
retval = sqlite3_exec(db, questionSqlStmt, getQuestionCallback, &qp,
&zErrMsg);
return retval;
}
static int getQuestionCallback(void *fpArg, int argc, char **argv, char
**azColName)
{
callbackArg = (struct questionPacket *)fpArg;
strcpy(callbackArg->id,argv[0]);
strcpy(callbackArg->data,argv[1]);
/* uncommenting line below causes segmentation fault */
/*strcpy(callbackArg->choiceA,argv[2]);
strcpy(callbackArg->choiceB,argv[3]);
strcpy(callbackArg->choiceC,argv[4]);
strcpy(callbackArg->choiceD,argv[5]);*/
return 0;
}
|
|
0
|
|
|
|
Reply
|
El_Durango (24)
|
12/4/2011 9:48:12 AM |
|
Le 04/12/2011 10:48, Durango a écrit :
> #define BUFFER_SIZ 128
Hummm.
> typedef struct answerPacket
> {
> char answer[1];
> }anspacket;
Humm. The answer int he struct is one char. This is probably a (commonly
used) trick to have a variable length array at the end of a structure.
The real size is most probably not 1.
Ie. the "anspacket" is probably just a string buffer of any size,
allocated with malloc(number_of_chars + sizeof(anspacket))
> int getQuestionPacket(struct questionPacket **qp);
> static int getQuestionCallback(void *fpArg, int argc, char **argv, char
> **azColName);
Yuk. Why a void* ? Use real types rather than having opaque types is
always better.
> int main()
> {
> qpacket *qp;
> anspacket *ap;
>
> int ret = -1;
>
> /*test*/
> createQuestionPacket((struct questionPacket **)&callbackArg);
A first advice: DO NOT USE CAST (especially pointers). Unless you
absolutely need them. And if you absolutely need them, ask yourself if
this is a good idea or not. (the only known legit case is unsigned char
vs. char due to historical POSIX oddities)
Here, you don't need any cast. &callbackArg is qpacket **.
> /* allocate packet memory */
> createQuestionPacket((struct questionPacket **)&qp);
createQuestionPacket should not allocate static-length buffers in
members. And why checking the first malloc return, if the other malloc
are unchecked ? :)
> (*qp)->id = malloc(sizeof(char) * 8);
> (*qp)->data = malloc(sizeof(char) * 256);
> (*qp)->choiceA = malloc(sizeof(char) * 128);
> (*qp)->choiceB = malloc(sizeof(char) * 128);
> (*qp)->choiceA = malloc(sizeof(char) * 128);
> (*qp)->choiceA = malloc(sizeof(char) * 128);
Use strdup() rather than allocating predefined buffer and strcpy them.
Or the program is going to blow up if argv[*] is too long.
> void createAnswerPacket(struct answerPacket **ap)
> {
> if((*ap=(struct answerPacket *)malloc(sizeof(anspacket)))==NULL)
(Here again a dirty cast). So you are allocating ONE char (which is
sufficient for the "" string). I'm wondering if this is going to be
sufficient for the answer :)
|
|
0
|
|
|
|
Reply
|
xroche (185)
|
12/4/2011 11:05:03 AM
|
|
Xavier Roche , dans le message <jbfk4v$n34$1@news.httrack.net>, a
�crit�:
> Yuk. Why a void* ? Use real types rather than having opaque types is
> always better.
Probably because of the prototype of sqlite3_exec.
|
|
0
|
|
|
|
Reply
|
george54 (194)
|
12/4/2011 11:13:47 AM
|
|
Durango <El_Durango@yahoo.com> writes:
> void createQuestionPacket(struct questionPacket **qp)
> {
> if((*qp=(struct questionPacket *)malloc(sizeof(qpacket)))==NULL)
> {
> fprintf(stderr, "No memory was allocated for change question packet!
> \n");
> exit(0);
> }
> (*qp)->id = malloc(sizeof(char) * 8);
> (*qp)->data = malloc(sizeof(char) * 256);
> (*qp)->choiceA = malloc(sizeof(char) * 128);
> (*qp)->choiceB = malloc(sizeof(char) * 128);
> (*qp)->choiceA = malloc(sizeof(char) * 128);
> (*qp)->choiceA = malloc(sizeof(char) * 128);
> }
Don't multiply by sizeof(char), it is 1.
The fixed-size buffers you're allocating here could more easily be
defined as arrays. That said, you'd be better off allocating them only
when you know how big they need to be.
> static int getQuestionCallback(void *fpArg, int argc, char **argv, char
> **azColName)
> {
> callbackArg = (struct questionPacket *)fpArg;
> strcpy(callbackArg->id,argv[0]);
> strcpy(callbackArg->data,argv[1]);
> /* uncommenting line below causes segmentation fault */
> /*strcpy(callbackArg->choiceA,argv[2]);
> strcpy(callbackArg->choiceB,argv[3]);
> strcpy(callbackArg->choiceC,argv[4]);
> strcpy(callbackArg->choiceD,argv[5]);*/
I imagine one of the argv[] entries is missing or too long.
--
http://www.greenend.org.uk/rjk/
|
|
0
|
|
|
|
Reply
|
rjk (492)
|
12/4/2011 11:14:38 AM
|
|
On Sun, 04 Dec 2011 11:13:47 +0000, Nicolas George wrote:
> Xavier Roche , dans le message <jbfk4v$n34$1@news.httrack.net>, a
> écrit :
>> Yuk. Why a void* ? Use real types rather than having opaque types is
>> always better.
>
> Probably because of the prototype of sqlite3_exec.
Yes, that's correct the api function sqlite3_exec I cannot do anything
about the arguments it takes.
|
|
0
|
|
|
|
Reply
|
El_Durango (24)
|
12/4/2011 11:20:56 AM
|
|
Durango <El_Durango@yahoo.com> writes:
> Hello I am working on a program that requires me to use structs. I am
> allocating memory for the struct and the variables defined in them.
> I am sure there is something incorrect with my code because there is
> undefined behavior when running the program.
What this supposed to mean? Someone else ran it and died before he
could inform you about the results?
|
|
0
|
|
|
|
Reply
|
rweikusat (2678)
|
12/4/2011 4:49:59 PM
|
|
Richard Kettlewell <rjk@greenend.org.uk> writes:
>Durango <El_Durango@yahoo.com> writes:
>
>> void createQuestionPacket(struct questionPacket **qp)
>> {
>> if((*qp=(struct questionPacket *)malloc(sizeof(qpacket)))==NULL)
>> {
>> fprintf(stderr, "No memory was allocated for change question packet!
>> \n");
>> exit(0);
>> }
>> (*qp)->id = malloc(sizeof(char) * 8);
>> (*qp)->data = malloc(sizeof(char) * 256);
>> (*qp)->choiceA = malloc(sizeof(char) * 128);
>> (*qp)->choiceB = malloc(sizeof(char) * 128);
>> (*qp)->choiceA = malloc(sizeof(char) * 128);
>> (*qp)->choiceA = malloc(sizeof(char) * 128);
>> }
>
>Don't multiply by sizeof(char), it is 1.
So what's wrong with using sizeof(char)? It is self-documenting,
correct, adds no run-time overhead, and helps supports architectures where
sizeof(char) != 1.
scott
|
|
0
|
|
|
|
Reply
|
scott
|
12/4/2011 7:07:36 PM
|
|
Durango <El_Durango@yahoo.com> writes:
>void createQuestionPacket(struct questionPacket **qp)
>{
> if((*qp=(struct questionPacket *)malloc(sizeof(qpacket)))==NULL)
> {
> fprintf(stderr, "No memory was allocated for change question packet!
>\n");
> exit(0);
> }
> (*qp)->id = malloc(sizeof(char) * 8);
> (*qp)->data = malloc(sizeof(char) * 256);
> (*qp)->choiceA = malloc(sizeof(char) * 128);
> (*qp)->choiceB = malloc(sizeof(char) * 128);
> (*qp)->choiceA = malloc(sizeof(char) * 128);
> (*qp)->choiceA = malloc(sizeof(char) * 128);
>}
did you not intend to allocate memory for choiceC and choiceD?
scott
|
|
0
|
|
|
|
Reply
|
scott
|
12/4/2011 7:09:30 PM
|
|
Le 04/12/2011 20:07, Scott Lurndal a écrit :
> So what's wrong with using sizeof(char)? It is self-documenting,
> correct, adds no run-time overhead, and helps supports architectures where
> sizeof(char) != 1.
A "typed" malloc (#define typed_malloc(TYPE,SIZE) (TYPE*)
malloc(sizeof(TYPE)*(SIZE))) might be clearer in such case.
[ BTW sizeof(char) is always 1.
<http://www.open-std.org/jtc1/sc22/WG14/www/docs/n1256.pdf>
[ sizeof ]
"When applied to an operand that has type char, unsigned char,or signed
char, (or a qualified version thereof) the result is 1."
]
|
|
0
|
|
|
|
Reply
|
xroche (185)
|
12/4/2011 7:16:36 PM
|
|
On 2011-12-04, Scott Lurndal <scott@slp53.sl.home> wrote:
> Richard Kettlewell <rjk@greenend.org.uk> writes:
>>
>>Don't multiply by sizeof(char), it is 1.
>
> So what's wrong with using sizeof(char)? It is self-documenting,
> correct, adds no run-time overhead, and helps supports architectures where
> sizeof(char) != 1.
There are no architectures where sizeof(char) != 1. The C standard
*defines* sizeof(char) to be 1.
Are you perhaps confused with the possibility that CHAR_BIT != 8? sizeof
does not give you the size in "bytes" or "octets" it gives you the size
in chars. So an architecture with CHAR_BIT == 32 where a single char is
32bits in length, still has sizeof(char) == 1.
--
John Tsiombikas
http://nuclear.mutantstargoat.com/
|
|
0
|
|
|
|
Reply
|
nuclear6 (33)
|
12/4/2011 10:08:43 PM
|
|
John Tsiombikas , dans le message
<slrnjdnrrb.nuc.nuclear@goat.mutantstargoat.com>, a �crit�:
> Are you perhaps confused with the possibility that CHAR_BIT != 8? sizeof
> does not give you the size in "bytes" or "octets" it gives you the size
> in chars.
Actually, it does give the size in byte, but the byte it refers to is
precisely the unit of memory necessary to store a char.
|
|
0
|
|
|
|
Reply
|
george54 (194)
|
12/4/2011 10:18:00 PM
|
|
|
10 Replies
39 Views
(page loaded in 0.166 seconds)
|