Programming Forums > Application Development > C

darthsabbath · May 23rd, 2006, 3:54 AM

I'm playing around with the classic K&R, and am in the chapter on Pointers. I'm having a bit of a problem with the strcopy() function listed... it keeps segfaulting whenever i compile and run the program. For those who don't have K&R, the code listed is:

(Toggle Plain Text)

/* Copies t to s */
void strcpy(char *s, char *t)
{
        while((*s = *t) != '\0') {
                s++;
                t++;
        }
}

The code I'm using to call the function is

(Toggle Plain Text)

main()
{
	char *p1; /* I've alternately tried pre-setting this to an equal length string to see if that would work... no joy */
	char *p2 = "Now is the time";
	strcpy(p1, p2);
}

The problem seems to be in the while((*s = *t) != '\0') loop. If I don't do the comparison between the two, I can manipulate the passed strings fine... i.e.

(Toggle Plain Text)

while(*t != '\0') {
	printf("%c\n", *t);
	s = t;
	t++;
	s++;
	printf("%c\n", *s);
	
}

I've also tried changing the while loop test to ((s = t) != '\0')... this results in the string being assigned properly... if I pipe out the contents of each char in the strings... I get

(Toggle Plain Text)

N N
o o
w w

i i
s s

t t
h h
e e

t t
i i
m m
e e
^@ ^@
% %
c c

% %
c c

It seems like it's going past the end of the string and into the realm of garbage.

I'm kinda lost, any pointers as to what I'm doing wrong (No pun intended)?

Thanks,
Phil

bivhitscar · May 23rd, 2006, 4:15 AM

(Toggle Plain Text)

main()
{
	char *p1;
	char *p2 = "Now is the time";
	strcpy(p1, p2);
}

This is bad. Think about what a pointer is - it is a variable that holds a memory address. In the case of p1, above, this is a character pointer which is two things. It is a pointer to NULL, or possibly garbage. It is also a pointer to one single char. I assume you haven't learnt about the malloc function yet, so I suggest rewriting the declaration *p1 as p1[100] ( I'll also assume you have done arrays, as you are now on pointers ).

strcpy() works like so:
Step one, send pointer to char (our target pointer, p1) and another pointer to char (our input pointer, p2). Take the value at the address of (in our case) p2 and assign it to the value at the address of p1. Stop here for a second, what is p1 pointing to? ( look up ). It is either pointing to NULL or garbage. You need to have allocated memory for p1, before you can start storing things there.

In your second example, you are just overwriting memory that is ( possibly ) already in use. This is undefined and can go unnoticed, crash later, or crash immediately depending on your luck.

Well, that post was longer than expected, hopefully it made some sense.

darthsabbath · May 23rd, 2006, 4:22 AM

~!

Awesome! I think I've got it now. :-) Thanks for the tip.

Damnit... it took forever for me to get OOP... I'm not going to give up on pointers either.

Phil

bivhitscar · May 23rd, 2006, 4:28 AM

Also, one other thing I saw you try:

(Toggle Plain Text)

((s = t) != '\0')

When testing the value of a pointer, you need to write it like this (assuming s and t are pointers):

(Toggle Plain Text)

((s = t) != NULL )

The \0 is the ascii representation of null, where NULL is referring to a null pointer. This is a little annoying, but you'll get it eventually.

darthsabbath · May 24th, 2006, 2:49 AM

Quote:

Originally Posted by bivhitscar

Also, one other thing I saw you try:

(Toggle Plain Text)

((s = t) != '\0')

When testing the value of a pointer, you need to write it like this (assuming s and t are pointers):

(Toggle Plain Text)

((s = t) != NULL )

The \0 is the ascii representation of null, where NULL is referring to a null pointer. This is a little annoying, but you'll get it eventually.

Just a question here... the code I was testing was from K&R, testing to see if the current character being pointed to is the string terminating character. In this instance, would one still use NULL?

Thanks for being patient everyone. :-)

Phil

grumpy · May 23rd, 2006, 6:22 AM

You might want to have a look at Dawei's page on pointers, here. Among other things, this page explains that declaring a pointer only declares the pointer. It does not magically, implicitly, or otherwise create something for that pointer to point at.

Or, to put it another way, if you don't explicitly initialise your pointer, it points at a random area of memory. So copying data to it (which strcpy() does to its first argument) is incredibly dangerous.

Jimbo · May 24th, 2006, 2:55 AM

somewhat simplified: NULL = 0 = '\0'

If I were doing it, I would just do something like:

(Toggle Plain Text)

while(!(*s = *t))

bivhitscar · May 24th, 2006, 5:01 AM

Quote:

Originally Posted by Jimbo

somewhat simplified: NULL = 0 = '\0'

Well, that is extremely simplified, because you can't do this:

char *ptr;

if ( ptr == '\0' );

To darksabbath:
If you want to check the value of a pointer, use NULL. If you want to check the value at the address a pointer points to, use '\0'.

In your first example while((*s = *t) != '\0') was correct and that's how I would do it. Jimbo's method is perfectly fine too.

[EDIT]

Though, I will mention that the statement while((*s = *t) != '\0') will overwrite memory before checking the value, which isn't very good practice IMO.

[EDIT v2]

And that's why there is a strncpy(), so while my first edit was valid, it is irrelevant with respect to the strcpy function.

I think I'll stop now...

Jimbo · May 24th, 2006, 5:38 AM

Quote:

Originally Posted by bivhitscar

Well, that is extremely simplified, because you can't do this:

(Toggle Plain Text)

char *ptr;
if ( ptr == '\0' );

Why not? It's a silly way of doing things, but what's stopping you?

Quote:

Originally Posted by bivhitscar

Though, I will mention that the statement while((*s = *t) != '\0') will overwrite memory before checking the value, which isn't very good practice IMO.

I wouldn't say that it's bad practice, as it really depends on the algorithm. For instance, for strcpy() you want it to copy the last value, so there's nothing wrong with this. It is good to point out that the assignment happens, but it's only bad if you're not expecting it to.

bivhitscar · May 25th, 2006, 5:58 AM

Quote:

Originally Posted by Jimbo

Why not? It's a silly way of doing things, but what's stopping you?

Well, it can be done - but the compiler should throw up a warning about a comparison between different types; in this case, a pointer and an integer.
Hmm, it doesn't seem to throw a warning for my last case, but it does for this:

(Toggle Plain Text)

int main(int argc, char *argv[])
{
    char * ptr = NULL;
    char ch = '\0';
    
    ptr = &ch;
    
    if ( *ptr == NULL )
    {
         printf("null\n");
    }
    
    system("PAUSE");	
    return 0;
}

Does this mean it is valid to compare a pointer with '\0', but not a character variable with NULL? So, NULL is only for pointers - but pointers can be used with either comparison. Stupid discrepancies.

Quote:

Originally Posted by Jimbo

I wouldn't say that it's bad practice, as it really depends on the algorithm.

And yeh, hence my second edit.

May 23rd, 2006, 3:54 AM	#1
darthsabbath Newbie Join Date: Dec 2005 Posts: 15 Rep Power: 0	Passing Pointers to Functions I'm playing around with the classic K&R, and am in the chapter on Pointers. I'm having a bit of a problem with the strcopy() function listed... it keeps segfaulting whenever i compile and run the program. For those who don't have K&R, the code listed is: (Toggle Plain Text) /* Copies t to s / void strcpy(char s, char t) { while((s = t) != '\0') { s++; t++; } } / Copies t to s / void strcpy(char s, char t) { while((s = t) != '\0') { s++; t++; } } The code I'm using to call the function is (Toggle Plain Text) main() { char p1; /* I've alternately tried pre-setting this to an equal length string to see if that would work... no joy / char p2 = "Now is the time"; strcpy(p1, p2); } main() { char p1; / I've alternately tried pre-setting this to an equal length string to see if that would work... no joy / char p2 = "Now is the time"; strcpy(p1, p2); } The problem seems to be in the while((s = t) != '\0') loop. If I don't do the comparison between the two, I can manipulate the passed strings fine... i.e. (Toggle Plain Text) while(t != '\0') { printf("%c\n", t); s = t; t++; s++; printf("%c\n", s); } while(t != '\0') { printf("%c\n", t); s = t; t++; s++; printf("%c\n", s); } I've also tried changing the while loop test to ((s = t) != '\0')... this results in the string being assigned properly... if I pipe out the contents of each char in the strings... I get (Toggle Plain Text) N N o o w w i i s s t t h h e e t t i i m m e e ^@ ^@ % % c c % % c c N N o o w w i i s s t t h h e e t t i i m m e e ^@ ^@ % % c c % % c c It seems like it's going past the end of the string and into the realm of garbage. I'm kinda lost, any pointers as to what I'm doing wrong (No pun intended)? Thanks, Phil

May 23rd, 2006, 4:15 AM	#2
bivhitscar Hobbyist Programmer Join Date: Oct 2005 Location: Melbourne, Australia Posts: 126 Rep Power: 4	(Toggle Plain Text) main() { char p1; char p2 = "Now is the time"; strcpy(p1, p2); } main() { char p1; char p2 = "Now is the time"; strcpy(p1, p2); } This is bad. Think about what a pointer is - it is a variable that holds a memory address. In the case of p1, above, this is a character pointer which is two things. It is a pointer to NULL, or possibly garbage. It is also a pointer to one single char. I assume you haven't learnt about the malloc function yet, so I suggest rewriting the declaration p1 as p1[100] ( I'll also assume you have done arrays, as you are now on pointers ). strcpy() works like so:* Step one, send pointer to char (our target pointer, p1) and another pointer to char (our input pointer, p2). Take the value at the address of (in our case) p2 and assign it to the value at the address of p1. Stop here for a second, what is p1 pointing to? ( look up ). It is either pointing to NULL or garbage. You need to have allocated memory for p1, before you can start storing things there. In your second example, you are just overwriting memory that is ( possibly ) already in use. This is undefined and can go unnoticed, crash later, or crash immediately depending on your luck. Well, that post was longer than expected, hopefully it made some sense. __________________ it's ironic considerate rarity patron of love higher knowledge engulfs me...

May 23rd, 2006, 4:28 AM	#4
bivhitscar Hobbyist Programmer Join Date: Oct 2005 Location: Melbourne, Australia Posts: 126 Rep Power: 4	Also, one other thing I saw you try: (Toggle Plain Text) ((s = t) != '\0') ((s = t) != '\0') When testing the value of a pointer, you need to write it like this (assuming s and t are pointers): (Toggle Plain Text) ((s = t) != NULL ) ((s = t) != NULL ) The \0 is the ascii representation of null, where NULL is referring to a null pointer. This is a little annoying, but you'll get it eventually. __________________ it's ironic considerate rarity patron of love higher knowledge engulfs me...

May 24th, 2006, 2:55 AM	#7
Jimbo Battle Programmer Join Date: Feb 2006 Location: Bellevue, WA, USA Posts: 770 Rep Power: 3	somewhat simplified: NULL = 0 = '\0' If I were doing it, I would just do something like: (Toggle Plain Text) while(!(s = t)) while(!(s = t))

Thread Tools
Show Printable Version Email this Page
Display Modes
Switch to Linear Mode Hybrid Mode Switch to Threaded Mode

May 23rd, 2006, 4:22 AM	#3
darthsabbath Newbie Join Date: Dec 2005 Posts: 15 Rep Power: 0	~! Awesome! I think I've got it now. :-) Thanks for the tip. Damnit... it took forever for me to get OOP... I'm not going to give up on pointers either. Phil

May 23rd, 2006, 6:22 AM	#6
grumpy Programming Guru Join Date: Jun 2005 Location: Adelaide, South Australia Posts: 1,260 Rep Power: 5	You might want to have a look at Dawei's page on pointers, here. Among other things, this page explains that declaring a pointer only declares the pointer. It does not magically, implicitly, or otherwise create something for that pointer to point at. Or, to put it another way, if you don't explicitly initialise your pointer, it points at a random area of memory. So copying data to it (which strcpy() does to its first argument) is incredibly dangerous.

Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)

Programming Forums > Application Development > C

Passing Pointers to Functions