memory leak purify

[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.

[Simon Richard Clarkstone]

sa*********@india.com wrote:

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

--
Simon Richard Clarkstone: s.************@durham.ac.uk/s*m*n.cl*rkst*n*@
hotmail.com ### "I have a spelling chequer / it came with my PC /
it plainly marks for my revue / Mistake's I cannot sea" ...
by: John Brophy (at: http://www.cfwf.ca/farmj/fjjun96/)

[Flash Gordon]

On 6 Dec 2004 23:55:33 -0800
sa*********@india.com wrote:

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.
--
Flash Gordon
Living in interesting times.
Although my email address says spam, it is real and I read it.

[Jens.Toerring]

Flash Gordon <sp**@flash-gordon.me.uk> wrote:

On 6 Dec 2004 23:55:33 -0800
sa*********@india.com wrote:

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
--
\ Jens Thoms Toerring ___ Je***********@physik.fu-berlin.de
\__________________________ http://www.toerring.de

[Charlie Gordon]

<sa*********@india.com> wrote in message
news:11**********************@f14g2000cwb.googlegr oups.com...

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

--
Chqrlie

[sangeetha_b]

thanks a lot for your help

Charlie Gordon wrote:

<sa*********@india.com> wrote in message
news:11**********************@f14g2000cwb.googlegr oups.com...

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

--
Chqrlie

[CBFalconer]

sa*********@india.com wrote:

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.

--
Chuck F (cb********@yahoo.com) (cb********@worldnet.att.net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home.att.net> USE worldnet address!