my strcpy() is not working

djneill

Hi all,
The shell program compiles under Linux fine, with -g so that any errors
would be displayed, but when the code is executed, the process ends
without error.

Here is the call:
strcpy( path, (const char *)tokens[0].c_str() );

path was declared with: char* path; but never initialized to
anything.
tokens[] is an STL vector of strings (not c-strings).
at the point of the call, tokens[0] = "/bin/ls"

So below is the context in which the call occurs. The child process is
spawned, the output looks like
"esh% /bin/ls
Input was a user command, about to fork
In the child process, building the args[]
about to build path
path is libc.so.6
esh%"

when it should continue
"just filled path with tokens[0]"

but it doesn't.
Context:
else if ( pid == 0) { // child process

cout << "In the child process, building the args[]" << endl;

cout << "about to build path" << endl;
cout << "tokens[0] is " << tokens[0].c_str() << endl;
char* path; // path for execl

cout << "path is " << path << endl;
// build path
// everything works fine up til here
strcpy( path, (const char *)tokens[0].c_str() );

// this line of code is never executed because the previous one
caused process
// to die
cout << "just filled path with tokens[0]: " << path << endl;
For any help, I would greatly appreciate it.

Thanks,
Dan

Oct 3 '06 #1

Victor Bazarov

dj*****@iwebworks.com wrote:

Hi all,
The shell program compiles under Linux fine, with -g so that any
errors would be displayed, but when the code is executed, the process
ends without error.

You may have a misconception about what -g means.

Here is the call:
strcpy( path, (const char *)tokens[0].c_str() );

There is no need to cast the result of calling c_str(). It is already
of type 'const char*'.

>
path was declared with: char* path; but never initialized to
anything.

If you pass an uninitalised pointer to 'strcpy', the behaviour of the
code is undefined. It may (allowed to) behave as if everything is OK.

tokens[] is an STL vector of strings (not c-strings).
at the point of the call, tokens[0] = "/bin/ls"

So below is the context in which the call occurs. The child process
is spawned, the output looks like
"esh% /bin/ls
Input was a user command, about to fork
In the child process, building the args[]
about to build path
path is libc.so.6
esh%"

when it should continue
"just filled path with tokens[0]"

but it doesn't.

The behaviour is undefined when you pass 'path' to 'cout' before
ever initialising it. 'cout' attempts to dereference the pointer
and that's where undefined behaivour occurs.

>

Context:
else if ( pid == 0) { // child process

cout << "In the child process, building the args[]" << endl;

cout << "about to build path" << endl;
cout << "tokens[0] is " << tokens[0].c_str() << endl;
char* path; // path for execl

cout << "path is " << path << endl;
// build path
// everything works fine up til here
strcpy( path, (const char *)tokens[0].c_str() );

// this line of code is never executed because the previous one
caused process
// to die
cout << "just filled path with tokens[0]: " << path << endl;
For any help, I would greatly appreciate it.

Make sure 'path' points to valid memory. Better yet, declare it
an *array* and not a pointer. And give enough room so the characters
would not be written beyond the place you allocate for them. Even
better, still, is to use 'string' everywhere and never touch bare
pointers unless you absolutely have to, and then still not touch them
if you don't know how they work (i.e. learn them before using them).

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

Oct 3 '06 #2

Frederick Gotham

strcpy( path, (const char *)tokens[0].c_str() );

Get rid of that cast -- it's redundant.

path was declared with: char* path; but never initialized to
anything.

Genius idea. As far as you're concerned, the pointer variable, "path",
contains a random memory address. You're writing to a random memory location.
Is that good, do you think?

You don't understand pointers. Here's a recent post of mine which may help:

http://groups.google.ie/group/comp.l...ddfb2a7?hl=en&

--

Frederick Gotham

Oct 3 '06 #3

Frederick Gotham

Victor Bazarov posted:

There is no need to cast the result of calling c_str(). It is already
of type 'const char*'.

And hypothetically speaking, even if it were "char*" rather than "char
const*", the cast would _still_ be redundant in the context of that function
call.

--

Frederick Gotham

Oct 3 '06 #4

djneill

Frederick Gotham wrote:

strcpy( path, (const char *)tokens[0].c_str() );

Get rid of that cast -- it's redundant.

path was declared with: char* path; but never initialized to
anything.

Genius idea. As far as you're concerned, the pointer variable, "path",
contains a random memory address. You're writing to a random memory location.
Is that good, do you think?

You don't understand pointers. Here's a recent post of mine which may help:

http://groups.google.ie/group/comp.l...ddfb2a7?hl=en&

--

Frederick Gotham

Hi all,
Thanks for your help. Indeed it is not good to randomly overwrite
whatever the uninitialized pointer is pointing to. Interesting is that
my first try worked on a Mac OS, but did not work on Linus which is why
I had to change it. Here is what I did to get the program to not
fault.

char* path; // path for execl
path = (char *)tokens[0].c_str();

Thanks,
Dan

Oct 3 '06 #5

djneill

my first try worked on a Mac OS, but did not work on Linus which is why

I had to change it.

Linux!

Sorry,
Dan

Oct 3 '06 #6

Victor Bazarov

dj*****@iwebworks.com wrote:

[..] Here is what I did to get the program to
not fault.

char* path; // path for execl
path = (char *)tokens[0].c_str();

WHY??? What for, FCOL?

Why couldn't you simply pass 'tokens[0]' to cout?

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

Oct 3 '06 #7

Frederick Gotham

char* path; // path for execl
path = (char *)tokens[0].c_str();

Ignoring the fact that there's no need for the invocation of c_str, I want to
talk to you about constness.

Please do me a little favour and compile the following:

int main()
{
char const *p = 0;

char *q = p;
}

Let me know how your compiler reacted... any errors, any warnings?

--

Frederick Gotham

Oct 3 '06 #8

djneill

Frederick Gotham wrote:

char* path; // path for execl
path = (char *)tokens[0].c_str();

Ignoring the fact that there's no need for the invocation of c_str, I want to
talk to you about constness.

Please do me a little favour and compile the following:

int main()
{
char const *p = 0;

char *q = p;
}

Let me know how your compiler reacted... any errors, any warnings?

--

Frederick Gotham

test.cpp: In function `int main()':
test.cpp:7: error: invalid conversion from `const char*' to `char*'
make: *** [test.o] Error 1

I was getting lots of that until I happened upon my final solution I
posted above. Maybe the c_str() is not necessary but the cast got it
into usable form.

Thanks,
Dan

Oct 3 '06 #9

Victor Bazarov

dj*****@iwebworks.com wrote:

Frederick Gotham wrote:

>>char* path; // path for execl
path = (char *)tokens[0].c_str();

Ignoring the fact that there's no need for the invocation of c_str,
I want to talk to you about constness.
[..]

I was getting lots of that until I happened upon my final solution I
posted above. Maybe the c_str() is not necessary but the cast got it
into usable form.

Usable for WHAT? What are you doing with that pointer after you obtain
it by const-casting the 'c_str' return value?

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

Oct 3 '06 #10

Thomas J. Gritzan

dj*****@iwebworks.com schrieb:

Frederick Gotham wrote:

>>char* path; // path for execl
path = (char *)tokens[0].c_str();

Ignoring the fact that there's no need for the invocation of c_str, I want to
talk to you about constness.

Please do me a little favour and compile the following:

int main()
{
char const *p = 0;

char *q = p;
}

Let me know how your compiler reacted... any errors, any warnings?

test.cpp: In function `int main()':
test.cpp:7: error: invalid conversion from `const char*' to `char*'
make: *** [test.o] Error 1

The compiler complains about an error and you tell him to shut up by using
a cast. Trust the compiler. Don't cast.

You can use a const char* or a std::string, maybe a std::string reference:

const char* path = tokens[0].c_str();

std::string path = tokens[0];

const std::string& path = tokens[0];

--
Thomas
http://www.netmeister.org/news/learn2quote.html

Oct 3 '06 #11