Best way of Allocating and Deallocating memory

[Amit_Basnak]

Dear Friends

I have two structures as below
typedef struct {
long_int length;
char data[1];
} CI_STRUCT_DATA;

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;

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

Thanks
Amit

 

[DaveJ]

Dear Friends

I have two structures as below
typedef struct {
long_int length;
char data[1];

} CI_STRUCT_DATA;

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;

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

Thanks
Amit

I tend to use classes over structs for most things. But anyway, I
think new and delete are the prefered methods for memory allocation in
C++. You don't need to provide the size parameter, and new returns a
pointer to the object your creating.

I would do something like:

class CIData
{
public:
CIData();
~CIData();

const long_int getLength() const;
const char* getData() const;

void setLength(long_int len);
void setData(const char*);

private:
long_int m_Length;
char[1] m_Data;
};

class CIMsg
{
public:
CIMsg();
~CIMsg();

// Setters
void setMsgData(CIData *);
// .....

// Getters
// ......

private:
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];
const CIData * m_msgData;
}


int main(int argc, char** argv)
{
const CIMsg msgDetails;
const CIData data = new CIData();

msgDetails.setMsgData(CIData);

if(msgDetails)
{
// do some stuff

...

delete msgDetails;
msgDetails = NULL;
}

return 0;
}

 

[Barry]

Dear Friends

I have two structures as below
typedef struct {
long_int length;
char data[1];
} CI_STRUCT_DATA;

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;

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

please format your code before post, it's so hard to read

 

[Alf P. Steinbach]

* Amit_Basnak:

Dear Friends

I have two structures as below
typedef struct {
long_int length;
char data[1];
} CI_STRUCT_DATA;

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;

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

Please format your code (indenting etc.) before you post.

Advice.

1. Reserve all uppercase for macros.
2. Use std::vector for memory allocation and deallocation.
3. Encapsulate operations as members.

Cheers, & hth.,

- Alf

 

[=?ISO-8859-1?Q?Erik_Wikstr=F6m?=]

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;

 

[Amit_Basnak]

Dear Friends

I have two structures as below
typedef struct {
long_int length;
char data[1];

} CI_STRUCT_DATA;

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;

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

Thanks
Amit

I tend to use classes over structs for most things. But anyway, I
think new and delete are the prefered methods for memory allocation in
C++. You don't need to provide the size parameter, and new returns a
pointer to the object your creating.

I would do something like:

class CIData
{
public:
CIData();
~CIData();

const long_int getLength() const;
const char* getData() const;

void setLength(long_int len);
void setData(const char*);

private:
long_int m_Length;
char[1] m_Data;

};

class CIMsg
{
public:
CIMsg();
~CIMsg();

// Setters
void setMsgData(CIData *);
// .....

// Getters
// ......

private:
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];
const CIData * m_msgData;

}

int main(int argc, char** argv)
{
const CIMsg msgDetails;
const CIData data = new CIData();

msgDetails.setMsgData(CIData);

if(msgDetails)
{
// do some stuff

...

delete msgDetails;
msgDetails = NULL;
}

return 0;



}- Hide quoted text -

- Show quoted text -- Hide quoted text -

- Show quoted text -

Thanks all

Since its mandator for us to use C style malloc
thats why I tried to allocate the memory like
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);

Now for freeing it up
free(msg_details.MSG_DATA);
Is above sufficient ?

Please let me know
Thanks

 

[James Kanze]

I have two structures as below
typedef struct {
long_int length;
char data[1];
} CI_STRUCT_DATA;

This looks like C. In C++, you wouldn't use a typedef here (and
in both languages, all caps is traditionally reserved for
macros).

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;

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)) );

I can't figure this line out at all. What's ptr_msg_details?

msg_details.MSG_DATA->length = strlen(ptr_msg_details);
strcpy((char *)msg_details.MSG_DATA->data,(char *)ptr_msg_details);

This last line is undefined behavior (in both C and C++) if
ptr_msg_details points to anything but an empty string. You've
declared CI_STRUCT_DATA::data as a char[1].

Im freeing up the memory as
free(ptr_msg_details);

Please suggest me any improvements in memory allocations if
any.

Without knowing what the structs are used for, it's impossible
to say. The code you have above, however, is not legal C++, nor
legal C.

 

[James Kanze]

On 2007-09-17 12:48, Amit_Basnak wrote:

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.

Actually, he treats it as if it were a char[], a VLA. C++ (like
C90) doesn't have VLA's, and the above isn't a VLA in C99
either; he'd have to declare it "char data[];".

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.

Unsigned is a bit special in C++, and in general, plain int
should be used as the default type, unless there are overriding
reasons for some other type. (One overriding reason is that
comparisons between signed and unsigned are sometimes confusing,
so if unsigned is imposed on you elsewhere---e.g. by the STL---,
then you probably want to conform to the type used there---in
the case of the STL, size_t.)

In C++, of course, the correct way to write the above would be:

typedef std::vector< char > CiData ;

Which solves all of his problems in one fell swoop.

[...]

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

Or std::vector< char >. I sort of suspect that he's trying to
format a message for transmission. In such cases, the
functionality of std::vector< char > is often more appropriate
than that of std::string.

But of course, I'm just guessing as to the use. All I really
know is that the code he posted looks more like C than C++, and
that it has undefined behavior in both languages.