memory leak purify

S

sangeetha_b

Hello,

I've writen one simple program to automate some manual process. I've
written that in c program. It works fine so far no problem reported on
this. Last week, i get chance to run my program on purify tool. I'm
very new to purify tool use.

Q1. Purify reports Uninitialized memory read in the following line A
and Line B

<<<<<<<<<<<<<<<<<<<<<<<<<<<<>>>>>>>>>>>>>>>>>>>>>>>>
int parameter_parse(char *ptr)
{
char *tmp;
PARAM *p_tmp;
p_tmp = (struct PARAM *)malloc(sizeof(struct PARAM));
if (!( tmp = strchr(ptr, '=') ))
printf("\nBailing out..");
exit (1);
}
tmp++; // since ptr includes delimiters too ("=")

p_tmp->value = strdup(tmp);
p_tmp->field =
(char *)malloc(strlen(ptr)-strlen(p_tmp->value)); // Line A
strncat(p_tmp->field, ptr,
(strlen(ptr)-strlen(p_tmp->value)-1)); // Line B
p_tmp->next = NULL;

if ( p_start == NULL)
{
p_start = p_tmp;
p_last = p_tmp;
} else {
p_last->next = p_tmp;
p_last = p_tmp;
}
return 0;
}
<<<<<<<<<<<<<<<<<<<<<<<<<<<<>>>>>>>>>>>>>>>>>>>>>>>>>
This "parameter_parse" funciton receives input in this form
"key=value". I dont know wht is the "Uninitialized memory read" memory
read at these two lines.

Q2. Similarly, it shows UMR in the following two lines
if ( t == NUMERIC )
strcat(res, NUMBER[9]); // Line C
else
strcat(res, ALPHABETS[9]); // Line D

Here, NUMBER and ALPHABETS are an array of 10 pointers-to-int.

Please help me to resolve this problem.
Thanks in advance.

Wluve,
Sangeetha.
 
S

Simon Richard Clarkstone

int parameter_parse(char *ptr)
{
char *tmp;
PARAM *p_tmp;
p_tmp = (struct PARAM *)malloc(sizeof(struct PARAM));
if (!( tmp = strchr(ptr, '=') ))
printf("\nBailing out..");
exit (1);
}
You do realise that this curly brace is the end of the function
parameter_parse(), don't you? You compiler *should* complain about a
"statement outside of any function" on the next line. If, however, your
program compiles and runs, make sure you have copied and pasted (or
imported into your mailer) the *actual code*.
tmp++; // since ptr includes delimiters too ("=")

p_tmp->value = strdup(tmp);
p_tmp->field =
(char *)malloc(strlen(ptr)-strlen(p_tmp->value)); // Line A
strncat(p_tmp->field, ptr,
(strlen(ptr)-strlen(p_tmp->value)-1)); // Line B
p_tmp->next = NULL;

if ( p_start == NULL)
{
p_start = p_tmp;
p_last = p_tmp;
} else {
p_last->next = p_tmp;
p_last = p_tmp;
}
return 0;
}
Presumably, that is where you *intended* the function to end.
If you run your code through an indenter, it would pick out the problem
immediately, and make your code considerably more legible (hint, hint).
 
F

Flash Gordon

On 6 Dec 2004 23:55:33 -0800
Hello,

I've writen one simple program to automate some manual process. I've
written that in c program. It works fine so far no problem reported on
this. Last week, i get chance to run my program on purify tool. I'm
very new to purify tool use.

Q1. Purify reports Uninitialized memory read in the following line A
and Line B

firstly, you need to get the code you post decently formatted using
spaces (not tabs) for indenting. I have reformatted your code below
since I need to in order to read it, but other than that the code marked
as quotation is unchanged.
<<<<<<<<<<<<<<<<<<<<<<<<<<<<>>>>>>>>>>>>>>>>>>>>>>>>
int parameter_parse(char *ptr)
{
char *tmp;
PARAM *p_tmp;
p_tmp = (struct PARAM *)malloc(sizeof(struct PARAM));

You don't need to cast the return value of malloc and it can hide the
very real error of failing to 'include <stdlib.h>. a far simpler and
less error prone form it:

p_tmp = malloc(sizeof *p_tmp);

if (!( tmp = strchr(ptr, '=') ))

{ /* otherwise it will always exit */
printf("\nBailing out..");
exit (1);

exit(1) is not portable. The only portable values are 0, EXIT_SUCCESS
and EXIT_FAILURE. So for portability this should be
exit(EXIT_FAILURE);
However, I accept that sometimes there are valid reasons for using
non-portable values.

Also, I think you retyped your code rather than copying and pasting,
since as written here this function will *always* call exit. Therefore
the error you are actually looking for is not exhibited by this code and
even after fixing this there could be other differences that are
relevant.

Always use copy and paste, *never* retype code for posting to usenet.
}
tmp++; // since ptr includes delimiters too ("=")

p_tmp->value = strdup(tmp);

strdup is a non-standard function. I happen to know what it does, but
you can't assume everyone here does and being non-standard it is off
topic here.
p_tmp->field =
(char *)malloc(strlen(ptr)-strlen(p_tmp->value)); // Line A

same comments about casting as in the previous example.
p_tmp->field = malloc(strlen(ptr)-strlen(p_tmp->value));

I can't see anything obviously wrong with this. However, it could be a
problem with the data passed in or you not typing what your actual code
is.
strncat(p_tmp->field, ptr,
(strlen(ptr)-strlen(p_tmp->value)-1)); // Line B

This, on the other hand, is definitely a problem. p_tmp->field points to
*uninitialised* memory and strncat obviously has to read the memory in
order to find the end of the string. This could make things go BANG big
time. However, since you know how much there is to copy (you worked it
out in order to allocate the memory) why not just use memcpy then set
the null termination on the string?
p_tmp->next = NULL;

if ( p_start == NULL)
{
p_start = p_tmp;
p_last = p_tmp;
} else {
p_last->next = p_tmp;
p_last = p_tmp;
}
return 0;
}
<<<<<<<<<<<<<<<<<<<<<<<<<<<<>>>>>>>>>>>>>>>>>>>>>>>>>
This "parameter_parse" funciton receives input in this form
"key=value". I dont know wht is the "Uninitialized memory read" memory
read at these two lines.

Q2. Similarly, it shows UMR in the following two lines
if ( t == NUMERIC )
strcat(res, NUMBER[9]); // Line C
else
strcat(res, ALPHABETS[9]); // Line D

Here, NUMBER and ALPHABETS are an array of 10 pointers-to-int.

Please help me to resolve this problem.
Thanks in advance.

You probably have not initialised res. If you look up the definition of
strcat you will see that the destination buffer obviously requires
initialising.
 
J

Jens.Toerring

Flash Gordon said:
On 6 Dec 2004 23:55:33 -0800
Q2. Similarly, it shows UMR in the following two lines
if ( t == NUMERIC )
strcat(res, NUMBER[9]); // Line C
else
strcat(res, ALPHABETS[9]); // Line D

Here, NUMBER and ALPHABETS are an array of 10 pointers-to-int. ^^^^^^^^^^^^^^^

Please help me to resolve this problem.
Thanks in advance.
You probably have not initialised res. If you look up the definition of
strcat you will see that the destination buffer obviously requires
initialising.

And, of course, using an int pointer as an argument to a string
handling function like strcat() isn't a good idea to start with.
If you want to convert an integer to a string use e.g. sprintf().

Regards, Jens
 
C

Charlie Gordon

Hello,

I've writen one simple program to automate some manual process. I've
written that in c program. It works fine so far no problem reported on
this. Last week, i get chance to run my program on purify tool. I'm
very new to purify tool use.

/* The code is not well formed, indentation should be consistent
* and only using spaces.
*/

/* Missing declarations here: */

#include <stdlib.h>
#include <stdio.h>
#include <string.h>

typedef struct PARAM PARAM;
struct PARAM {
PARAM *next;
char *field;
char *value;
};

PARAM *p_start, *p_last;
int parameter_parse(char *ptr)

since parameter_parse does not modify its input buffer, define it as const
{
char *tmp;
PARAM *p_tmp;

poor choice of names. no information carried.
p_tmp = (struct PARAM *)malloc(sizeof(struct PARAM));

useless cast. error prone even.
allocation failure not tested.
if (!( tmp = strchr(ptr, '=') ))

missing {
this code cannot compile as is.
bad style.
printf("\nBailing out..");

uninformative error message.
exit (1);

unportable exit() parameter value
}
tmp++; // since ptr includes delimiters too ("=")

do not use // comments
the comment itself is awkward, just say that tmp points to '='
with a short notice like skip '='
p_tmp->value = strdup(tmp);

non standard function, but a fine addition ;-)
no test for memory allocation failure.
p_tmp->field =
(char *)malloc(strlen(ptr)-strlen(p_tmp->value)); // Line A

useless cast, allocation too short by 1 byte !
it beats me why other forum regulars do not catch this!
cumbersome and slow computation of "field" length.
strncat(p_tmp->field, ptr,
(strlen(ptr)-strlen(p_tmp->value)-1)); // Line B

redundant computation of "field" length.
strncat() to an uninitialized buffer.
one more example of strncat() misuse. function should be deprecated.
p_tmp->next = NULL;

if ( p_start == NULL)
{
p_start = p_tmp;
p_last = p_tmp;
} else {
p_last->next = p_tmp;
p_last = p_tmp;
}

inconsistent style
return 0;

constant return value.
Why not return an error code instead of "bailing out" ?

---------------

/* Here is an example with most issues fixed: */

int parameter_parse(const char *ptr)
{
char *sep;
PARAM *p_param;

p_param = malloc(sizeof(*p_param));
if (!p_param) {
fprintf(stderr, "cannot allocate memory for PARAM structure\n");
exit(EXIT_FAILURE);
}
if ((sep = strchr(ptr, '=')) == NULL) {
fprintf(stderr, "invalid parameter line: %s\n", ptr);
exit(EXIT_FAILURE);
}

p_param->field = calloc(sep - ptr + 1, sizeof(char));
p_param->value = calloc(strlen(sep + 1) + 1, sizeof(char));
p_param->next = NULL;

if (!p_param->field || !p_param->value) {
fprintf(stderr, "cannot allocate memory for PARAM members\n");
free(p_param->field);
free(p_param->value);
free(p_param);
exit(EXIT_FAILURE);
}

memcpy(p_param->field, ptr, sep - ptr);
strcpy(p_param->value, sep + 1);

if (p_start == NULL) {
p_start = p_last = p_param;
} else {
p_last->next = p_param;
p_last = p_param;
}
return 0;
}
 
S

sangeetha_b

thanks a lot for your help

Charlie said:
/* The code is not well formed, indentation should be consistent
* and only using spaces.
*/

/* Missing declarations here: */

#include <stdlib.h>
#include <stdio.h>
#include <string.h>

typedef struct PARAM PARAM;
struct PARAM {
PARAM *next;
char *field;
char *value;
};

PARAM *p_start, *p_last;


since parameter_parse does not modify its input buffer, define it as const

poor choice of names. no information carried.


useless cast. error prone even.
allocation failure not tested.


missing {
this code cannot compile as is.
bad style.


uninformative error message.


unportable exit() parameter value


do not use // comments
the comment itself is awkward, just say that tmp points to '='
with a short notice like skip '='


non standard function, but a fine addition ;-)
no test for memory allocation failure.


useless cast, allocation too short by 1 byte !
it beats me why other forum regulars do not catch this!
cumbersome and slow computation of "field" length.


redundant computation of "field" length.
strncat() to an uninitialized buffer.
one more example of strncat() misuse. function should be deprecated.


inconsistent style


constant return value.
Why not return an error code instead of "bailing out" ?


---------------

/* Here is an example with most issues fixed: */

int parameter_parse(const char *ptr)
{
char *sep;
PARAM *p_param;

p_param = malloc(sizeof(*p_param));
if (!p_param) {
fprintf(stderr, "cannot allocate memory for PARAM structure\n");
exit(EXIT_FAILURE);
}
if ((sep = strchr(ptr, '=')) == NULL) {
fprintf(stderr, "invalid parameter line: %s\n", ptr);
exit(EXIT_FAILURE);
}

p_param->field = calloc(sep - ptr + 1, sizeof(char));
p_param->value = calloc(strlen(sep + 1) + 1, sizeof(char));
p_param->next = NULL;

if (!p_param->field || !p_param->value) {
fprintf(stderr, "cannot allocate memory for PARAM members\n");
free(p_param->field);
free(p_param->value);
free(p_param);
exit(EXIT_FAILURE);
}

memcpy(p_param->field, ptr, sep - ptr);
strcpy(p_param->value, sep + 1);

if (p_start == NULL) {
p_start = p_last = p_param;
} else {
p_last->next = p_param;
p_last = p_param;
}
return 0;
}
 
C

CBFalconer

thanks a lot for your help

I have my doubts that you will be getting much more, after this
silly and rude top-posting and failure to snip 160 odd lines that
have no connection to your posting.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads


Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top