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[i]);

bin[i]=bin3;

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

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

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 <stdio.h>

#include "string.h"
why "string.h"? <string.h> is enough. 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[i]);

bin[i]=bin3;

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

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

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]

priya <pr*************@gmail.com> wrote:

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[i]);
bin3 points to a newly allocated string with the contents of var[i].
bin[i]=bin3;
Now bin[i] points to the *same* newly allocated string.
printf("\n the bin3 value is %s",bin3);
printf("\n the bin value is %s",bin[i]);
free(bin3);
bin3 and bin[i] 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.

--
Christopher Benson-Manica | I *should* know what I'm talking about - if I
ataru(at)cyberspace.org | don't, I need to know. Flames welcome.

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

In article <op***************@mohur.intern.dresearch.de>,
Jirka Klaue <jk****@tkn.tu-berlin.de> wrote:

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]

James Daughtry wrote:

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

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson

[Keith Thompson]

"Jirka Klaue" <jk****@tkn.tu-berlin.de> writes:

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.

[...]

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

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[Martin Ambuhl]

priya wrote:

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
#include <string.h>
?
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[i]);
bin[i] = bin3;
printf("the bin3 value is %s\n", bin3);
printf("the bin value is %s\n", bin[i]);
}
checking(bin);
for (i = 0; i <= 1; i++) /* mha: moved */
free(bin[i]);
return 0; /* mha: added */
}

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

[Anonymous 7843]

In article <11**********************@g43g2000cwa.googlegroups .com>,
James Daughtry <mo*******@hotmail.com> wrote:

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)

^^^^^^ (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]

In article <43***************@yahoo.com>,
CBFalconer <cb********@worldnet.att.net> wrote:

James Daughtry wrote:

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

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
--
The recipe says "toss lightly," but I suppose that depends
on how much you eat and how bad the cramps get. - J. Lileks

[Mark]

"Gary E. Ansok" <an***@alumni.caltech.edu> wrote in message
news:de**********@naig.caltech.edu...

In article <43***************@yahoo.com>,
CBFalconer <cb********@worldnet.att.net> wrote:

James Daughtry wrote:

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

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]

In article <PZJOe.677$mH.257@fed1read07>,
Anonymous 7843 <an******@example.com> wrote:

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

an******@example.com (Anonymous 7843) writes:

In article <PZJOe.677$mH.257@fed1read07>,
Anonymous 7843 <an******@example.com> wrote:

...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 (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[Keith Thompson]

an***@alumni.caltech.edu (Gary E. Ansok) writes:

In article <43***************@yahoo.com>,
CBFalconer <cb********@worldnet.att.net> wrote:

James Daughtry wrote:

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

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

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[pete]

Keith Thompson wrote:

an***@alumni.caltech.edu (Gary E. Ansok) writes:

In article <43***************@yahoo.com>,
CBFalconer <cb********@worldnet.att.net> wrote:

James Daughtry wrote:

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

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.

--
pete

[Mac]

On Tue, 23 Aug 2005 16:39:10 +0000, Richard Tobin wrote:

In article <op***************@mohur.intern.dresearch.de>,
Jirka Klaue <jk****@tkn.tu-berlin.de> wrote:

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

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]

Keith Thompson <ks***@mib.org> writes:

an***@alumni.caltech.edu (Gary E. Ansok) writes:

In article <43***************@yahoo.com>,
CBFalconer <cb********@worldnet.att.net> wrote:

James Daughtry wrote:

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

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]

Tim Rentsch wrote:

Keith Thompson <ks***@mib.org> writes:

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

I like Keith's better.

--
pete

[Tim Rentsch]

pete <pf*****@mindspring.com> writes:

Tim Rentsch wrote:

Keith Thompson <ks***@mib.org> writes:

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

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.

[Gordon Burditt]

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

I like X better than Y because X is more like Y than Y is like X.

Gordon L. Burditt

[Anton Petrusevich]

CBFalconer wrote:

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

Great! But why not to scan src only once?

char *dupstr(const char *src)
{
size_t len;
char *dst;
if((dst = malloc((len = strlen(src) + 1))))
memcpy(dst, src, len);
return dst;
}

--
Anton Petrusevich

[Keith Thompson]

Anton Petrusevich <ca***@att-ltd.biz> writes:

CBFalconer wrote:

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

Great! But why not to scan src only once?

char *dupstr(const char *src)
{
size_t len;
char *dst;
if((dst = malloc((len = strlen(src) + 1))))
memcpy(dst, src, len);
return dst;
}

That still scans src twice, once for strlen() and once for memcpy().

strcpy() scans until it sees a '\0'. memcpy() scans for a specified
number of bytes. It's not at all obvious that one is going to be
faster than the other (though one or the other could well be on some
particular platform).

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[pete]

Tim Rentsch wrote:

pete <pf*****@mindspring.com> writes:

Tim Rentsch wrote:

Keith Thompson <ks***@mib.org> writes:

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

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?

Keith's is simpler.

--
pete

[Mark B]

"pete" <pf*****@mindspring.com> wrote in message
news:43***********@mindspring.com...

Tim Rentsch wrote:

pete <pf*****@mindspring.com> writes:

> Tim Rentsch wrote:
> > Keith Thompson <ks***@mib.org> writes:
> > > 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;
> > }
>
> 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?

Keith's is simpler.

They are both 'simple'... the usage of the conditional operator
in the example is not complex.

My personal preference however would be to see the != NULL
and braces used in Keith's condition dropped.
Then I too would like Keith's better for no other reason than
it's how I would have coded it. :-)

As for 2nd example: 2 spaces after return, but none between
the arguments to strcpy()? And why the blank line in the middle?
Sloppy? Maybe. Definately not complicated. Either could be
used as an acceptable solution as they get the job done
efficiently and properly.

Mark

[pete]

Mark B wrote:

"pete" <pf*****@mindspring.com> wrote in message
news:43***********@mindspring.com...

Tim Rentsch wrote:

pete <pf*****@mindspring.com> writes:
> Tim Rentsch wrote:
> > Keith Thompson <ks***@mib.org> writes:
> > > 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;
> > }
>
> 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?
Keith's is simpler.

They are both 'simple'... the usage of the conditional operator
in the example is not complex.

My personal preference however would be to see the != NULL

I like the != NULL.
When you see
(p != NULL)
then you have enough context to know that p is a pointer,
without looking anywhere else.
and braces used in Keith's condition dropped.
I prefer compound statements with all if, else, and loop, statements.
For reasons similar to these:
http://groups.google.com/group/comp....30c019b?hl=en&
Then I too would like Keith's better for no other reason than
it's how I would have coded it. :-)

As for 2nd example: 2 spaces after return, but none between
the arguments to strcpy()? And why the blank line in the middle?
Sloppy?

I prefer to separate declarations from statements
with a blank line in compound statements
because it's a common convention.

--
pete

[Mark B]

"pete" <pf*****@mindspring.com> wrote in message
news:43***********@mindspring.com...

Mark B wrote:

"pete" <pf*****@mindspring.com> wrote in message
news:43***********@mindspring.com...

> Tim Rentsch wrote:
>> pete <pf*****@mindspring.com> writes:
>> > Tim Rentsch wrote:
>> > > Keith Thompson <ks***@mib.org> writes:
>> > > > 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;
>> > > }
>> >
>> > 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?
>
> Keith's is simpler.

They are both 'simple'... the usage of the conditional operator
in the example is not complex.

My personal preference however would be to see the != NULL

I like the != NULL.
When you see
(p != NULL)
then you have enough context to know that p is a pointer,
without looking anywhere else.

You have enough context to know that p 'should be' a pointer
without having to look anywhere else... doesn't necessarily
mean that it is though, does it? But even if you could ascertain
that it was in fact a pointer (which I don't know how you can do
without a peek at the definition) what does it point to?
What good is knowing you have a pointer if you don't know
what it points to? Don't you still need to familiarize yourself
with the code before making any modifications? Doesn't that
still entail a peek at the definition?

and braces used in Keith's condition dropped.

I prefer compound statements with all if, else, and loop, statements.
For reasons similar to these:
http://groups.google.com/group/comp....30c019b?hl=en&

(refers to a post made by akarl on 8/22 - applicable portions quoted)

Regarding advantage 1:
* Bugs are avoided (multiple statements on one line, the classical
* else-if situation, wrong indentation...)

I don't put multiple statements on one line, always indent properly, and
there
is no classical else-if situation as I do use braces when necessary...
(when the compiler bitches about something when I have warning turned up
I deem it necessary ;)

Regarding advantage 2:
* If you use braces only when necessary, you have to decide if they are
* required or not at every control statement you write. If you add one
* statement to a single statement you have to add the braces, and if you
* remove one of two statements you probably want to remove the braces as
* well to achieve consistency throughout the code. This requires some
* extra editing. Since all choices in programming are distracting, the
* irrelevant ones should be removed.

I typically know when I'm coding if I intend to write one or many statements
before I even begin to form the condition... by that point the decision has
already been made. If my requirements later change and I have to add
(or remove) a line I don't find it distracting to do so... though I do find
it
distracting to know that some coders consider 'all choices in programming'
to be distracting! Maybe they should familiarize themselves with the code
a little more before making changes. You should know what your
modifications
are going to do before you make them, no?

Regarding advantage 3:
* With explicit control statement termination the language gets less
* complicated and more regular.

Claim doesn't make sense. There is 'explicit control statement termination'
regardless of whether or not braces are used. In one instance it terminates
with the brace... in the other it terminates after the statement(s).
Shouldn't be any ambiguity there.

Regarding disadvantage 1 (and only):
* The code gets somewhat more cluttered and slightly less readable in
* case of short control statements.

I agree :-)

Then I too would like Keith's better for no other reason than
it's how I would have coded it. :-)

As for 2nd example: 2 spaces after return, but none between
the arguments to strcpy()? And why the blank line in the middle?
Sloppy?

I prefer to separate declarations from statements
with a blank line in compound statements
because it's a common convention.

Yet you liked Keith's code which omitted it? :-)
In general I also tend to seperate my declarations from the first
statement with a blank line, but for no other reason than for
code aesthetics (yes, I think it looks nice).

But: if I'm writing a 2 line function which consists of:
1) a function call to initialize a variable in its declaration
2) a return statement

I consider 1 to be a statement which no longer requires
a blank line to seperating it from the only other statement
in the function... call me silly :-)

Mark

[Joe Wright]

Gordon Burditt wrote:

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.

I like X better than Y because X is more like Y than Y is like X.

Gordon L. Burditt

You know Y is better than X. You just like to fight.
--
Joe Wright
"Everything should be made as simple as possible, but not simpler."
--- Albert Einstein ---

[Mac]

On Mon, 29 Aug 2005 19:52:17 +0000, Keith Thompson wrote:

Anton Petrusevich <ca***@att-ltd.biz> writes:

CBFalconer wrote:

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

Great! But why not to scan src only once?

char *dupstr(const char *src)
{
size_t len;
char *dst;
if((dst = malloc((len = strlen(src) + 1))))
memcpy(dst, src, len);
return dst;
}

That still scans src twice, once for strlen() and once for memcpy().

strcpy() scans until it sees a '\0'. memcpy() scans for a specified
number of bytes. It's not at all obvious that one is going to be
faster than the other (though one or the other could well be on some
particular platform).

Well, it is VERY hard for me to imagine that strcpy() will execute faster
than memcpy() when the string length is known ahead of time. I think in
general, memcpy() will be as fast as the implementers know how to make it.

But from the standard's perspective, I guess there is no reason to favor
either over the other.

--Mac

[Walter Roberson]

In article <pa****************************@bar.net>, Mac <fo*@bar.net> wrote:

On Mon, 29 Aug 2005 19:52:17 +0000, Keith Thompson wrote:

strcpy() scans until it sees a '\0'. memcpy() scans for a specified
number of bytes. It's not at all obvious that one is going to be
faster than the other (though one or the other could well be on some
particular platform).

Well, it is VERY hard for me to imagine that strcpy() will execute faster
than memcpy() when the string length is known ahead of time. I think in
general, memcpy() will be as fast as the implementers know how to make it.

A memcpy() implementation could potentially detect the case where
both memory areas had the same word alignment, copy a few bytes to
reach the aligning point, and then copy words (or longwords) until
it reached near the end and then copied bytes again. This would,
for example, work if both areas happened to be the beginning of
malloc()'d memory, as malloc() uses the most restrictive alignment.
It could do similarily for the case where the areas could be aligned
with shorter words... even moving by short int might be faster than
moving by byte.

Unfortunately, in a lot of cases, the two areas will not share any
mutual alignment, such as if one is copying starting at an "odd" memory
location and the source starts at an "even" location. And in that
particular case, strcpy() is potentially faster. When the byte
is moved, many architectures will automatically "test" the byte
value, allowing a "branch on zero" without the overhead of an
explicit test. That can be faster than decrementing a counter
location (which requires a write operation, even if only a
register write) and branching on the implicitly-tested value of it --
it takes one less operation, since one has moved the byte anyhow...
--
Any sufficiently old bug becomes a feature.

[pete]

Mark B wrote:

"pete" <pf*****@mindspring.com> wrote in message

I prefer to separate declarations from statements
with a blank line in compound statements
because it's a common convention.

Yet you liked Keith's code which omitted it? :-)

Yes.
I didn't say it was perfect.
In general I also tend to seperate my declarations from the first
statement with a blank line, but for no other reason than for
code aesthetics (yes, I think it looks nice).

But: if I'm writing a 2 line function which consists of:
1) a function call to initialize a variable in its declaration
2) a return statement

I consider 1 to be a statement
which no longer requires
a blank line to seperating it from the only other statement
in the function... call me silly :-)

Then you are wrong.
Declarations and definitions and are not statements.

--
pete

[Mark B]

"pete" <pf*****@mindspring.com> wrote in message
news:43**********@mindspring.com...

Mark B wrote:

"pete" <pf*****@mindspring.com> wrote in message

> I prefer to separate declarations from statements
> with a blank line in compound statements
> because it's a common convention.

Yet you liked Keith's code which omitted it? :-)

Yes.
I didn't say it was perfect.

In general I also tend to seperate my declarations from the first
statement with a blank line, but for no other reason than for
code aesthetics (yes, I think it looks nice).

But: if I'm writing a 2 line function which consists of:
1) a function call to initialize a variable in its declaration
2) a return statement

I consider 1 to be a statement
which no longer requires
a blank line to seperating it from the only other statement
in the function... call me silly :-)

Then you are wrong.
Declarations and definitions and are not statements.

Nor did I mean to claim that it actually was a statement...
I was implying that I would treat that particular declaration as if it were
a statement.
In retrospect, I should have left the last part of the post off completely
as I'd already
(appropriately) stated that for me personally I separate declarations from
statements
for code aesthetics, and no other reason. When it looks better with a blank
line, it
gets a blank line...

Can I assume (based on the fact that you snipped about 90% of my previous
post)
that you agreed with everything else I had written?

Mark

[Tim Rentsch]

pete <pf*****@mindspring.com> writes:

Tim Rentsch wrote:

pete <pf*****@mindspring.com> writes:

Tim Rentsch wrote:
>
> Keith Thompson <ks***@mib.org> writes:

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

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?

Keith's is simpler.

Is this some objective metric that you're applying,
or just your personal subjective metric? Most of
the metrics I'm used to would rank the second
definition as simpler.

[Mac]

On Tue, 30 Aug 2005 04:59:07 +0000, Walter Roberson wrote:

In article <pa****************************@bar.net>, Mac <fo*@bar.net> wrote:

On Mon, 29 Aug 2005 19:52:17 +0000, Keith Thompson wrote:

strcpy() scans until it sees a '\0'. memcpy() scans for a specified
number of bytes. It's not at all obvious that one is going to be
faster than the other (though one or the other could well be on some
particular platform).

Well, it is VERY hard for me to imagine that strcpy() will execute faster
than memcpy() when the string length is known ahead of time. I think in
general, memcpy() will be as fast as the implementers know how to make it.

A memcpy() implementation could potentially detect the case where
both memory areas had the same word alignment, copy a few bytes to
reach the aligning point, and then copy words (or longwords) until
it reached near the end and then copied bytes again. This would,
for example, work if both areas happened to be the beginning of
malloc()'d memory, as malloc() uses the most restrictive alignment.
It could do similarily for the case where the areas could be aligned
with shorter words... even moving by short int might be faster than
moving by byte.

Unfortunately, in a lot of cases, the two areas will not share any
mutual alignment, such as if one is copying starting at an "odd" memory
location and the source starts at an "even" location. And in that
particular case, strcpy() is potentially faster. When the byte
is moved, many architectures will automatically "test" the byte
value, allowing a "branch on zero" without the overhead of an
explicit test. That can be faster than decrementing a counter
location (which requires a write operation, even if only a
register write) and branching on the implicitly-tested value of it --
it takes one less operation, since one has moved the byte anyhow...

I'm not going to enter into this kind of argument here. Anyone who cares
which way is faster should profile both ways. Enough said.

;-)

--Mac

[Tim Rentsch]

Keith Thompson <ks***@mib.org> writes:

Anton Petrusevich <ca***@att-ltd.biz> writes:

CBFalconer wrote:

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

Great! But why not to scan src only once?

char *dupstr(const char *src)
{
size_t len;
char *dst;
if((dst = malloc((len = strlen(src) + 1))))
memcpy(dst, src, len);
return dst;
}

That still scans src twice, once for strlen() and once for memcpy().

strcpy() scans until it sees a '\0'. memcpy() scans for a specified
number of bytes. It's not at all obvious that one is going to be
faster than the other (though one or the other could well be on some
particular platform).

Of course, what Anton meant was that strcpy has to "scan" the
string in the sense of examining the values to see where the
terminating zero is; and that memcpy doesn't have to compare
values to zero, only move them. The entry for "scan" at m-w.com
gives:

to examine by point-by-point observation or checking

It seems reasonable to say that strcpy does this examining/checking
and that memcpy doesn't.

As it appears that Anton has learned English not as his first
language, that makes it more likely that he would use the word in
the sense that a dictionary defines it.

[Keith Thompson]

Tim Rentsch <tx*@alumnus.caltech.edu> writes:

Keith Thompson <ks***@mib.org> writes:

Anton Petrusevich <ca***@att-ltd.biz> writes:

> CBFalconer wrote:
>> 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;
>> }
>
> Great! But why not to scan src only once?
>
> char *dupstr(const char *src)
> {
> size_t len;
> char *dst;
> if((dst = malloc((len = strlen(src) + 1))))
> memcpy(dst, src, len);
> return dst;
> }

That still scans src twice, once for strlen() and once for memcpy().

strcpy() scans until it sees a '\0'. memcpy() scans for a specified
number of bytes. It's not at all obvious that one is going to be
faster than the other (though one or the other could well be on some
particular platform).

Of course, what Anton meant was that strcpy has to "scan" the
string in the sense of examining the values to see where the
terminating zero is; and that memcpy doesn't have to compare
values to zero, only move them. The entry for "scan" at m-w.com
gives:

to examine by point-by-point observation or checking

It seems reasonable to say that strcpy does this examining/checking
and that memcpy doesn't.

As it appears that Anton has learned English not as his first
language, that makes it more likely that he would use the word in
the sense that a dictionary defines it.

As strcpy() traverses the string, it has to check each element to see
whether it's equal to '\0'.

As memcpy() traverses the string, it has to check for each element
whether it's reached the number of bytes it was asked to copy.

My point was simply that it's not clear that either is going to be
faster than the other; I wasn't quibbling about the meaning of "scan".

--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[Tim Rentsch]

Keith Thompson <ks***@mib.org> writes:

Tim Rentsch <tx*@alumnus.caltech.edu> writes:

Keith Thompson <ks***@mib.org> writes:

Anton Petrusevich <ca***@att-ltd.biz> writes:
> CBFalconer wrote:
>> 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;
>> }
>
> Great! But why not to scan src only once?
>
> char *dupstr(const char *src)
> {
> size_t len;
> char *dst;
> if((dst = malloc((len = strlen(src) + 1))))
> memcpy(dst, src, len);
> return dst;
> }

That still scans src twice, once for strlen() and once for memcpy().

strcpy() scans until it sees a '\0'. memcpy() scans for a specified
number of bytes. It's not at all obvious that one is going to be
faster than the other (though one or the other could well be on some
particular platform).
Of course, what Anton meant was that strcpy has to "scan" the
string in the sense of examining the values to see where the
terminating zero is; and that memcpy doesn't have to compare
values to zero, only move them. The entry for "scan" at m-w.com
gives:

to examine by point-by-point observation or checking

It seems reasonable to say that strcpy does this examining/checking
and that memcpy doesn't.

As it appears that Anton has learned English not as his first
language, that makes it more likely that he would use the word in
the sense that a dictionary defines it.

As strcpy() traverses the string, it has to check each element to see
whether it's equal to '\0'.

Right, strcpy() has to check with each char transferred.

As memcpy() traverses the string, it has to check for each element
whether it's reached the number of bytes it was asked to copy.
The memcpy() code doesn't have to check after each char. Because
memcpy() knows the length ahead of time, units larger than char
units can be transferred at once. Standard loop unrolling stuff.
There still will be loop tests, but very likely fewer of them.

My point was simply that it's not clear that either is going to be
faster than the other; I wasn't quibbling about the meaning of "scan".

Of course, there's no guarantee that memcpy() will run faster than
strcpy() on all platforms, but wouldn't you expect it to run faster
on most platforms? Certainly I would, at least for lengths more
than five or ten characters.

Just as a datapoint, timing on an x86 showed dupstr()-using-strcpy()
running behind dupstr()-using-memcpy(), anywhere from about 1%
slower to about 75% slower, depending on how many characters were
being moved.

[pete]

Tim Rentsch wrote:

pete <pf*****@mindspring.com> writes:

Tim Rentsch wrote:

pete <pf*****@mindspring.com> writes:

> Tim Rentsch wrote:
> >
> > Keith Thompson <ks***@mib.org> writes:
>
> > > 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;
> > }
>
> 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?

Keith's is simpler.

Is this some objective metric that you're applying,
or just your personal subjective metric? Most of
the metrics I'm used to would rank the second
definition as simpler.

Keith's only decides whether or not to call strcpy.
The other one also decides which value to return.

--
pete