Problem in Strdup()

[priya]

Hi all,

I am using strdup() in my c program..But I am having some
pr0blem while using the free() in my c code.here I am pasting the my
code.




#include <stdio.h>
#include "string.h"
int checking(char *a[20]);
void main()
{

int i;
char *bin3;
char *bin[20];

char *var[20];
var[0]="hello";
var[1]="world";

for( i=0;i<=1;i++)

{

bin3= strdup(var);

bin=bin3;

printf("\n the bin3 value is %s",bin3);

printf("\n the bin value is %s",bin);

free(bin3);

}
checking(bin);


}
int checking (char *a[20])
{
printf("\n the valus is %s %s ",a[0],a[1]);
return 0;
}



My output is

the bin3 value is hello

the bin value is world

the valus is ---------

I am not getting the final value inside checking function...




But i am getting the final output when comment the free(bin3) method.

//free(bin3)


I could not find the problem.if any one know the solution,plz let me
know.


Thanks...

 

[manoj1978]

#include "string.h"

int checking(char *a[20]);
void main()

make it int main(void)

{

int i;
char *bin3;
char *bin[20];

char *var[20];
var[0]="hello";
var[1]="world";

for( i=0;i<=1;i++)

{

bin3= strdup(var);

bin=bin3;

printf("\n the bin3 value is %s",bin3);

printf("\n the bin value is %s",bin);

free(bin3);

strdup allocate memory and returns a pointer
you just freed the pointer.

}
checking(bin);

what will be in bin[0] and bin[1]?
two freed pointers.
accessing memory through a freed memory is like selling a house and
later try to stay in that house.

add a return 0; here.
}
int checking (char *a[20])
{
printf("\n the valus is %s %s ",a[0],a[1]);
return 0;
}



My output is

the bin3 value is hello

the bin value is world

the valus is ---------

I am not getting the final value inside checking function...




But i am getting the final output when comment the free(bin3) method.

//free(bin3)


I could not find the problem.if any one know the solution,plz let me
know.


Thanks...

strdup is available only in unix.so not portable also.
regards,
manoj.

 

[James Daughtry]

strdup is available only in unix.so not portable also.

True, it's not portable, but strdup is available on other systems if
the implementation provides it. For example, Microsoft's compiler
supports it. Of course, it's trivial to write, so the whole portability
issue is moot:

char *dupstr(const char *src)
{
char *p = malloc (strlen(src) + 1);

if (p == NULL)
return NULL;

strcpy(p, src);

return p;
}

 

[Christopher Benson-Manica]

void main()

main() returns int: http://www.eskimo.com/~scs/C-faq/q11.12.html

Prefer

int main( void )

char *var[20];
var[0]="hello";
var[1]="world";

Consider

const char *var[]={ "hello", "world" };

for( i=0;i<=1;i++)
{
bin3= strdup(var);

bin3 points to a newly allocated string with the contents of var.

bin=bin3;

Now bin points to the *same* newly allocated string.

printf("\n the bin3 value is %s",bin3);
printf("\n the bin value is %s",bin);
free(bin3);

bin3 and bin now point to memory that has been freed...

}
checking(bin);

....which checking() attempts to access; this is a Bad Thing. Once a
pointer has been passed to free(), the memory it pointed to is off
limits.

 

[Jirka Klaue]

James Daughtry:

char *dupstr(const char *src)
{
char *p = malloc (strlen(src) + 1);

if (p == NULL)
return NULL;

strcpy(p, src);

If you keep the malloced size you could use memcpy instead.
That would save you an implicit strlen.

return p;
}

Jirka

 

[Richard Tobin]

If you keep the malloced size you could use memcpy instead.
That would save you an implicit strlen.

Can you be sure that using a counter is faster than looking for a
zero in the data?

-- Richard

 

[CBFalconer]

True, it's not portable, but strdup is available on other systems
if the implementation provides it. For example, Microsoft's
compiler supports it. Of course, it's trivial to write, so the
whole portability issue is moot:

char *dupstr(const char *src)
{
char *p = malloc (strlen(src) + 1);

if (p == NULL)
return NULL;

strcpy(p, src);

return p;
}

Which brings up a style point. Why create multiple exit points
unnecessarily, instead of simply making the copy conditional on
suitable conditions. i.e.:

char *dupstr(const char *src)
{
char *p;

if (p = malloc(strlen(src) + 1)) strcpy(p, src);
return p;
}

 

[Keith Thompson]

James Daughtry:


If you keep the malloced size you could use memcpy instead.
That would save you an implicit strlen.

[...]

There's no implicit strlen() in strcpy(). strcpy() only has to scan
the source string once.

 

[Martin Ambuhl]

Hi all,

I am using strdup() in my c program.

Be warned that strdup() is not part of the standard C libraries and is
off topic here. It is not ISO (ANSI) C, and was not, last I checked,
even POSIX.

Also note that main returns an int. Anyone posting to a newsgroup
should always follow the newsgroup and check the FAQ before posting.
Only a very perverse person could use "void main" in posting if they had
done these things. That leads to the conclusion that you either are
purposely perverse or failed to follow simple usenet etiquette before
posting.

Further, your use of
#include "string.h"
raises a flag. Do you have a personal copy of string.h that you are
using rather than the standard one, for which the normal use is

But I am having some
pr0blem while using the free() in my c code.here I am pasting the my
code.

Note that free() is prototyped in <stdlib.h>, which you failed to include.

You are calling free() at the wrong place. You are freeing memory which
you then try to use later.

Here is a start at fixing your problems:

#include <stdio.h>
#include <stdlib.h> /* mha: added */
#include <string.h> /* mha: fixed */

char *strdup(const char *source); /* mha: added. This is a common
prototype for the non-standard
function strdup(). No library
is required to provide such a
function and, even if yours
does, there is no guarantee that
this prototype is correct for
it. */

/* mha: all the 'end-of-line' characters have been moved from the
perverse 'beginning-of-line' position. This avoids the OP's
nonportable behavior of the last line of output. */

int /* mha: fixed */ main(void)
{
int i;
char *bin3;
char *bin[2];
char *var[2];
int checking(char *a[2]); /* mha: moved */
var[0] = "hello";
var[1] = "world";
for (i = 0; i <= 1; i++) {
bin3 = strdup(var);
bin = bin3;
printf("the bin3 value is %s\n", bin3);
printf("the bin value is %s\n", bin);
}
checking(bin);
for (i = 0; i <= 1; i++) /* mha: moved */
free(bin);
return 0; /* mha: added */
}

int checking(char *a[2])
{
printf("the value is %s %s\n", a[0], a[1]);
return 0;
}

 

[Anonymous 7843]

True, it's not portable, but strdup is available on other systems if
the implementation provides it. For example, Microsoft's compiler
supports it. Of course, it's trivial to write, so the whole portability
issue is moot:

char *dupstr(const char *src)

^^^^^^ (pointing at dupstr, for those not using monospace fonts)

I'd like to emphasize the subtle point implied by James here.

In c89 (possibly in c99 also, haven't checked yet) you cannot
legally name your strdup-like function strdup as it infringes on
the implementation's ownership of the str[a-z][A-Z0-9_] namespace.

In actual practice, if you supply your own strdup function it
may create a link error on platforms which already have a strdup
function in their C library.

 

[Gary E. Ansok]

Which brings up a style point. Why create multiple exit points
unnecessarily, instead of simply making the copy conditional on
suitable conditions. i.e.:

char *dupstr(const char *src)
{
char *p;

if (p = malloc(strlen(src) + 1)) strcpy(p, src);
return p;
}

Personally, I find the first one much easier to understand at a
glance. Others will disagree, no doubt.

I've never been a big fan of the "one and only one return from
a function" theory -- to me, "something went wrong, bail out
as soon as possible" is more in tune with how I want to handle
things like invalid parameters and allocation failures.

To me, `if (condition) return` adds less complexity than
`if (condition) {code}` -- each one gives two traces through
the function, but in the first case, one trace is easily understood.
(If the code in the block is long, it can be hard to find where
the condition-false trace restarts -- but that's a general
problem with long code blocks.)

There are exceptions, of course, as there are more actions to
be unwound and they get more complicated. In a function
process_file(), I would probably return early from a failure to
open the file, but handle errors during the read in-line.

Besides, the first form makes it much easier to add additional
actions that might be needed in case of failure (not likely in
this specific example, of course) -- printing an error message,
freeing other resources, etc.

Gary

 

[Mark]

Personally, I find the first one much easier to understand at a
glance. Others will disagree, no doubt.

Yes, though I'd prefer to see the condition on a line by itself
in the 2nd example.

I've never been a big fan of the "one and only one return from
a function" theory -- to me, "something went wrong, bail out
as soon as possible" is more in tune with how I want to handle
things like invalid parameters and allocation failures.

I am. The programmers who 'bail out as soon as possible' are
often sloppy, they'll leave open file descriptors laying around,
don't free allocated memory, and often consider a c program
to consist of 2 or 3 1000 line functions. I've had to rewrite many
utilities because of such problems.

To me, `if (condition) return` adds less complexity than
`if (condition) {code}` -- each one gives two traces through
the function, but in the first case, one trace is easily understood.
(If the code in the block is long, it can be hard to find where
the condition-false trace restarts -- but that's a general
problem with long code blocks.)

Not hard to find if you properly indent your code
Harder to find the actual point of exit when you have 100 return
statements in a function all returning the same status.
I've seen that done before too.

There are exceptions, of course, as there are more actions to
be unwound and they get more complicated. In a function
process_file(), I would probably return early from a failure to
open the file, but handle errors during the read in-line.

if((fp = fopen(file, "r")) != NULL) {

fclose(fp);
}
No exceptions... that's the fashion in which I start every time.

Besides, the first form makes it much easier to add additional
actions that might be needed in case of failure (not likely in
this specific example, of course) -- printing an error message,
freeing other resources, etc.

Easier? I don't know about easier... with proper indentation and
commenting I find it is very 'easy' to do those things.

Mark

 

[Anonymous 7843]

...implementation's ownership of the str[a-z][A-Z0-9_] namespace.

Begging everyone's pardon, please allow me to restate
that as: str[a-z][A-Za-z0-9_]*

 

[Keith Thompson]

...implementation's ownership of the str[a-z][A-Z0-9_] namespace.

Begging everyone's pardon, please allow me to restate
that as: str[a-z][A-Za-z0-9_]*

Or, given the constraint that the name must be a valid identifier:
str[a-z].* (assuming regular expression syntax).

 

[Keith Thompson]

Personally, I find the first one much easier to understand at a
glance. Others will disagree, no doubt.

I've never been a big fan of the "one and only one return from
a function" theory -- to me, "something went wrong, bail out
as soon as possible" is more in tune with how I want to handle
things like invalid parameters and allocation failures.

[snip]

How about this?

char *dupstr(const char *src)
{
char *p = malloc(strlen(src) + 1);
if (p != NULL) {
strcpy(p, src);
}
return p;
}

The advantage is that there's only one exit point. The disadvantage
is that the return statement logically does one of two things,
depending on whether the malloc() succeeded. But it makes it clear
that dupstr() returns the same value as malloc() and for the same
reason (i.e., depending on whether the memory was successfully
allocated).

 

[pete]

Personally, I find the first one much easier to understand at a
glance. Others will disagree, no doubt.

I've never been a big fan of the "one and only one return from
a function" theory -- to me, "something went wrong, bail out
as soon as possible" is more in tune with how I want to handle
things like invalid parameters and allocation failures.

[snip]

How about this?

char *dupstr(const char *src)
{
char *p = malloc(strlen(src) + 1);
if (p != NULL) {
strcpy(p, src);
}
return p;
}

The advantage is that there's only one exit point.

I like it.

 

[Mac]

Can you be sure that using a counter is faster than looking for a
zero in the data?

-- Richard

Realistically, on short strings it won't matter. On long strings, it is
very unlikely that anything in the standard library will beat memcpy().

Back when we were discussing a clc library, the clc_strdup implementation
used memcpy(), for whatever that is worth. I just did a quick google
groups search, so here it is:

#include <string.h>

char *clc_strdup(const char *s)
{
size_t size = strlen(s) + 1;
char *p = malloc(size);
if (p != NULL) memcpy(p, s, size);
return p;
}

--Mac

ps, I think this was pretty much Dan Pop's version of strdup, but if there
are any errors, they are my fault.

 

[Tim Rentsch]

Personally, I find the first one much easier to understand at a
glance. Others will disagree, no doubt.

I've never been a big fan of the "one and only one return from
a function" theory -- to me, "something went wrong, bail out
as soon as possible" is more in tune with how I want to handle
things like invalid parameters and allocation failures.

[snip]

How about this?

char *dupstr(const char *src)
{
char *p = malloc(strlen(src) + 1);
if (p != NULL) {
strcpy(p, src);
}
return p;
}

char *dupstr(const char *src)
{
char *p = malloc(strlen(src) + 1);

return p ? strcpy(p,src) : 0;
}

 

[pete]

char *dupstr(const char *src)
{
char *p = malloc(strlen(src) + 1);

return p ? strcpy(p,src) : 0;
}

I like Keith's better.

 

[Tim Rentsch]

I like Keith's better.

That's all well and good, but what qualities allow
the first to be judged better than the second?
What is your metric? For all we know, you like
Keith's version better because the author's name
starts with a letter that comes earlier in the
alphabet.

I appreciate the opportunity to learn something
new, but statements to the effect of "I like X
better than Y" without any other explanation
rarely afford the opportunity to learn much of
anything beyond the statement itself.