On 2007-09-17 12:48, Amit_Basnak wrote:
Dear Friends
I have two structures as below
typedef struct {
long_int length;
char data[1];
} CI_STRUCT_DATA;
Notice that char data[1] is an array with one element, which is the same
as a normal char, then later your treat it as if it was a char pointer.
This is *very* bad, a char is 1 byte, a pointer is probably at least 4
times that big, which means that you are trying to write to memory
outside of the struct. Also, you use a type long_int, is that a typedef
for a long? Further more, typedefs with the structs are not needed in
C++ and can be omitted. Lastly, all upper-case names are generally
reserved for macros.
struct CiData {
long length;
char* data;
};
Note that length should probably be unsigned, and a normal int is
probably enough.
>
typedef CI_STRUCT_DATA *ptr_CiStructData;
typedef struct {
char opener_ref_num_fmtd[26];
char our_ref_num[15];
char txn_ref_num[15];
char stop_tracer_date[27];
char tracer_trans_medium[3];
char tracer_frequency[2];
ptr_CiStructData MSG_DATA;
} CI_STRUCT_MSG;
struct CiMsg {
char opener_ref_num_fmtd[26];
char our_ref_num[15];
char txn_ref_num[15];
char stop_tracer_date[27];
char tracer_trans_medium[3];
char tracer_frequency[2];
CiData* MSG_DATA;
};
Much of the above could probably better be represented by some other
format, such as integers instead of char arrays.
I have allocated memory for it as below
CI_STRUCT_MSG msg_details;
msg_details.MSG_DATA = (CI_STRUCT_DATA *)
malloc( sizeof(CI_STRUCT_DATA) +(sizeof(unsigned
char)*(strlen(ptr_msg_details) + 1)) );
msg_details.MSG_DATA->length = strlen(ptr_msg_details);
strcpy((char *)msg_details.MSG_DATA->data,(char *)ptr_msg_details);
Im freeing up the memory as
free(ptr_msg_details);
Please suggest me any improvements in memory allocations if any
Using new:
CiMsg msg_details;
msg_details.MSG_DATA = new CiData;
msg_details.MSG_DATA->data = new char[strlen(ptr_msg_details) + 1];
strcpy(msg_details.MSG_DATA->data, ptr_msg_details);
To free the memory use delete:
delete msg_details.MSG_DATA->data;
detele msg_details.MSG_DATA;
An even better solution would be to use std::string
struct CiMsg {
char opener_ref_num_fmtd[26];
char our_ref_num[15];
char txn_ref_num[15];
char stop_tracer_date[27];
char tracer_trans_medium[3];
char tracer_frequency[2];
std::string data;
};
// ...
CiMsg msg_details;
msg_details.assign(ptr_msg_details, strlen(ptr_msg_details));
Of course, if ptr_msg_details was also pointing to a string it would be
even simpler
CiMsg msg_details;
msg_details.data = *ptr_msg_data;
--
Erik Wikström