Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Problem with strcat, strcpy,sprintf

Reply
Thread Tools

Problem with strcat, strcpy,sprintf

 
 
diego.arias.vel@gmail.com
Guest
Posts: n/a
 
      10-28-2005
Hi all

I have certain problem when I'm doing this:

void copy(char *filename)
{
char *log_file;
log_file=filename;
strcat(log_file,"-log.txt");

//.......
}

suppose that filename="myfile.dat"

I'm expecting:
log_file="myfile.dat-log.txt"
and this work fine....

the problem is that I need to remain filename as the original name but
instead i have:
filename="myfile.dat-log.txt"

How can I do to avoid this, and preserve the original name???

Thanks!!

 
Reply With Quote
 
 
 
 
Arctic Fidelity
Guest
Posts: n/a
 
      10-28-2005
On Fri, 28 Oct 2005 17:09:43 -0400, <> wrote:

> void copy(char *filename)
> {
> char *log_file;
> log_file=filename;
> strcat(log_file,"-log.txt");
>
> //.......
> }
>
> suppose that filename="myfile.dat"
>
> I'm expecting:
> log_file="myfile.dat-log.txt"
> and this work fine....
>
> the problem is that I need to remain filename as the original name but
> instead i have:
> filename="myfile.dat-log.txt"
>
> How can I do to avoid this, and preserve the original name???


When you assign the pointer of filename (assuming that's how you properly
declared that), to log_file, then you're telling log_file to point to the
same space in memory that filename is. If you use strcat, you're feeding
that pointer into the function, and it is concatenating information to
that point. Since both filename and log_file are pointing to the same
space in memory, the same space is going to be written.

What you want to do is create a copy of the memory pointed to by filename,
and assign log_file to the copy. That is, you have to have two different
spaces in memory, so that you can copy the space pointed to by filename to
the space pointed to by log_file, and then you can change the space
pointed to by log_file without changing the space of filename, because
filename is pointing to another space. Make sense?

Think:

#include <string.h>
#define STRGSIZE 50

void main(void)
{
char *log_file;
char filename[] = "test";

log_file = malloc(STRGSIZE);

strncat(log_file, "-log.txt", STRGSIZE);
printf("filename: %s\nlog_file: %s\n", filename, log_file);
}

- Arctic

--
Using Opera's revolutionary e-mail client: http://www.opera.com/mail/
 
Reply With Quote
 
 
 
 
Keith Thompson
Guest
Posts: n/a
 
      10-28-2005
"Arctic Fidelity" <> writes:
> On Fri, 28 Oct 2005 17:09:43 -0400, <> wrote:
>
>> void copy(char *filename)
>> {
>> char *log_file;
>> log_file=filename;
>> strcat(log_file,"-log.txt");
>>
>> //.......
>> }
>>
>> suppose that filename="myfile.dat"
>>
>> I'm expecting:
>> log_file="myfile.dat-log.txt"
>> and this work fine....
>>
>> the problem is that I need to remain filename as the original name but
>> instead i have:
>> filename="myfile.dat-log.txt"
>>
>> How can I do to avoid this, and preserve the original name???

>
> When you assign the pointer of filename (assuming that's how you
> properly declared that), to log_file, then you're telling log_file to
> point to the same space in memory that filename is. If you use
> strcat, you're feeding that pointer into the function, and it is
> concatenating information to that point. Since both filename and
> log_file are pointing to the same space in memory, the same space is
> going to be written.
>
> What you want to do is create a copy of the memory pointed to by
> filename, and assign log_file to the copy. That is, you have to have
> two different spaces in memory, so that you can copy the space
> pointed to by filename to the space pointed to by log_file, and then
> you can change the space pointed to by log_file without changing the
> space of filename, because filename is pointing to another
> space. Make sense?


That's basically correct, but there are some serious problems in your
code. You should try compiling and executing it before posting.

> Think:
>
> #include <string.h>


Since you use printf(), you also need a "#include <stdio.h>".
Since you use malloc(), you also need a "#include <stdlib.h>".

> #define STRGSIZE 50


Why 50, especially since you can figure out exactly how much space is
actually needed?

> void main(void)


No, no, no, no, no.

main() returns int, not void.

> {
> char *log_file;
> char filename[] = "test";
>
> log_file = malloc(STRGSIZE);


Always check the result of malloc(); if it fails, it will return a
null pointer. Often the only thing you can do in response is to abort
the program, but it's better than continuing blindly.

You're trying to concatenate two strings. You know the length of each
of them, therefore you know exactly how much space you need for the
concatenation.

At this point, log_file points to an uninitialized block of 50 bytes.
There's no guarantee that this block contains a valid string, so
passing it to strncat() invokes undefined behavior.

The value of log_file is supposed to be "test-log.txt", but you never
copy the value "test" into log_file.

> strncat(log_file, "-log.txt", STRGSIZE);
> printf("filename: %s\nlog_file: %s\n", filename, log_file);
> }


Finally, you should have a "return 0;" at the end of your main
function. It's not required in C99, but it can't hurt, and it's
considered good style.

Try this:

#include <string.h>
#include <stdio.h>
#include <stdlib.h>

int main(void)
{
const char *filename = "test";
const char *suffix = "-log.txt";
char *log_file;
size_t log_file_len = strlen(filename) + strlen(suffix) + 1;

log_file = malloc(log_file_len);
if (log_file == NULL) {
fprintf(stderr, "malloc failed\n");
exit(EXIT_FAILURE);
}

strcpy(log_file, filename);
strcat(log_file, suffix);

printf("filename = \"%s\"\n", filename);
printf("suffix = \"%s\"\n", suffix);
printf("log_file = \"%s\"\n", log_file);

return 0;
}

Note that the original question assumed a function that takes the
filename as an argument; neither your program nor my modified version
of it does this. Probably the function should take a char* argument
and return a char* result. Returning a dynamically sized string can
be complicated; either you have to assume a maximum size, or the
caller has to allocate the space (which can be difficult if the caller
doesn't know how much space will be required), or the function has to
allocate the space (making the caller responsible for deallocating
it). I'm going to leave the code as it is for now, but the original
poster should feel free to ask followup questions.

--
Keith Thompson (The_Other_Keith) kst- <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.
 
Reply With Quote
 
Mark McIntyre
Guest
Posts: n/a
 
      10-28-2005
On 28 Oct 2005 14:09:43 -0700, in comp.lang.c ,
wrote:

>Hi all
>
>I have certain problem when I'm doing this:


you probably have a couple...

>void copy(char *filename)
>{
> char *log_file;
> log_file=filename;


This points log_file to the same place as filename.

Remember that in C, = is not the copy operator, its the assignment
operator. For pointer types, this sets the pointers to point to the
same place. It does /not/ copy the contents.

> strcat(log_file,"-log.txt");


Then you try to append to it. In other words, you're appending to the
/original string/, not a copy of it

By the way, is filename large enough to store 8 extra characters?
Better make sure, or this will crash.

>suppose that filename="myfile.dat"


if you defined it as
char *filename = "myfile.dat";
then its not only too small, but nonmodifiable.

>I'm expecting:
> log_file="myfile.dat-log.txt"
>and this work fine....
>
>the problem is that I need to remain filename as the original name but
>instead i have:
> filename="myfile.dat-log.txt"
>
>How can I do to avoid this, and preserve the original name???


Copy the filename to a new variable via the strcpy function or one of
its friends.
--
Mark McIntyre
CLC FAQ <http://www.eskimo.com/~scs/C-faq/top.html>
CLC readme: <http://www.ungerhu.com/jxh/clc.welcome.txt>

----== Posted via Newsfeeds.Com - Unlimited-Uncensored-Secure Usenet News==----
http://www.newsfeeds.com The #1 Newsgroup Service in the World! 120,000+ Newsgroups
----= East and West-Coast Server Farms - Total Privacy via Encryption =----
 
Reply With Quote
 
diego.arias.vel@gmail.com
Guest
Posts: n/a
 
      10-28-2005
hi:

Thanks to all......

I understand now about my error. I was a little confused.

My final code is working very well, even if filename is taken by the
function as argument...

void copy(char *filename)
{
//......
char *log_file;
const char *suffix = "-log.txt";

size_t log_file_len = strlen(filename) + strlen(suffix) + 1;
log_file = malloc(log_file_len);
if (log_file == NULL) { perror("malloc - log_file"); exit(1); }

strcpy(log_file, filename);
strcat(log_file, suffix);

printf("\n\nlog_file: %s\nfilename: %s\n\n", log_file,filename);

//....
}

Again, thanks to all, specially to keith!!!!

 
Reply With Quote
 
Keith Thompson
Guest
Posts: n/a
 
      10-28-2005
Mark McIntyre <> writes:
> On 28 Oct 2005 14:09:43 -0700, in comp.lang.c ,
> wrote:

[...]
>>void copy(char *filename)
>>{
>> char *log_file;
>> log_file=filename;

>
> This points log_file to the same place as filename.
>
> Remember that in C, = is not the copy operator, its the assignment
> operator. For pointer types, this sets the pointers to point to the
> same place. It does /not/ copy the contents.

[...]

I don't think I'd phrase it that way.

C's assignment operator, "=", is a copy operator. In the case of
"log_file=filename", it's copying the value of filename into the
variable log_file. The value happens to be a pointer value, so the
effect of this copy is that log_file points to the same place as
filename. This is just like a pointer assignment in just about any
other procedural language that has pointers.

Another way to put it is that "=" does a shallow copy, not a deep
copy; it copies only the value itself, not anything that it might
point to.

What's unusual about C is that the assignment operator can't be used
on arrays. Arrays in C are almost always manipulated indirectly,
through pointers; they're not treated as first-class types. (We could
argue for weeks about what "first-class types" means; let's not.) So
a lot of things that look like they're operating on arrays (in this
case, strings) are really operating on pointers, and thus aren't
necessarily doing what you might expect.

That's why the library provides functions like strcpy() and memcpy()
to do things that might be done by simple assignment statements in
other languages.

Section 6 of the C FAQ has some good information on arrays and
pointers.

--
Keith Thompson (The_Other_Keith) kst- <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.
 
Reply With Quote
 
Arctic Fidelity
Guest
Posts: n/a
 
      10-28-2005
On Fri, 28 Oct 2005 18:34:47 -0400, Keith Thompson <kst-> wrote:

>> What you want to do is create a copy of the memory pointed to by
>> filename, and assign log_file to the copy. That is, you have to have
>> two different spaces in memory, so that you can copy the space
>> pointed to by filename to the space pointed to by log_file, and then
>> you can change the space pointed to by log_file without changing the
>> space of filename, because filename is pointing to another
>> space. Make sense?

>
> That's basically correct, but there are some serious problems in your
> code. You should try compiling and executing it before posting.


Whoops...ehe *sheepish grin of embarassment* I've just been made a fool
of. I was writing too quickly and not thinking quite straight. I
guess I was thinking "illustration" without thinking, "Will this work?"

>> #include <string.h>

>
> Since you use printf(), you also need a "#include <stdio.h>".
> Since you use malloc(), you also need a "#include <stdlib.h>".


Bah! Doi! *hits head* Stupid, stupid, stupid [me]!

>> #define STRGSIZE 50

>
> Why 50, especially since you can figure out exactly how much space is
> actually needed?


I was just hoping to reduce the total number of operations and
instructions that I was putting in to try to eliminate extra brain
usage...obviously that didn't work.

>> void main(void)

>
> No, no, no, no, no.
>
> main() returns int, not void.


:-O I never knew...Wah?? Gah! Ouch. I'll keep that in definite mind next
time.

>> {
>> char *log_file;
>> char filename[] = "test";
>>
>> log_file = malloc(STRGSIZE);

>
> Always check the result of malloc(); if it fails, it will return a
> null pointer. Often the only thing you can do in response is to abort
> the program, but it's better than continuing blindly.


I was just trying to elminate writing more code... Heh...my bad.

> You're trying to concatenate two strings. You know the length of each
> of them, therefore you know exactly how much space you need for the
> concatenation.


Exactly true...Hmm...I guess I skipped over that in my haste.

> At this point, log_file points to an uninitialized block of 50 bytes.
> There's no guarantee that this block contains a valid string, so
> passing it to strncat() invokes undefined behavior.


DOI! Oh, the idiocy that is me! I should have seen that...*shudder*

> The value of log_file is supposed to be "test-log.txt", but you never
> copy the value "test" into log_file.


*blinks* *checks pulse* I think my brain is not working right tonight...

>> strncat(log_file, "-log.txt", STRGSIZE);
>> printf("filename: %s\nlog_file: %s\n", filename, log_file);
>> }

>
> Finally, you should have a "return 0;" at the end of your main
> function. It's not required in C99, but it can't hurt, and it's
> considered good style.


Naturally, with the int main(void) declaration that only makes sense.

> Try this:
>
> #include <string.h>
> #include <stdio.h>
> #include <stdlib.h>
>
> int main(void)
> {
> const char *filename = "test";
> const char *suffix = "-log.txt";
> char *log_file;
> size_t log_file_len = strlen(filename) + strlen(suffix) + 1;
>
> log_file = malloc(log_file_len);
> if (log_file == NULL) {
> fprintf(stderr, "malloc failed\n");
> exit(EXIT_FAILURE);
> }
>
> strcpy(log_file, filename);
> strcat(log_file, suffix);
>
> printf("filename = \"%s\"\n", filename);
> printf("suffix = \"%s\"\n", suffix);
> printf("log_file = \"%s\"\n", log_file);
>
> return 0;
> }
>
> Note that the original question assumed a function that takes the
> filename as an argument; neither your program nor my modified version
> of it does this. Probably the function should take a char* argument
> and return a char* result. Returning a dynamically sized string can
> be complicated; either you have to assume a maximum size, or the
> caller has to allocate the space (which can be difficult if the caller
> doesn't know how much space will be required), or the function has to
> allocate the space (making the caller responsible for deallocating
> it). I'm going to leave the code as it is for now, but the original
> poster should feel free to ask followup questions.



The lesson learned: test, compile, run, and then debug your code before
posting it! :-/

- Arctic

--
Using Opera's revolutionary e-mail client: http://www.opera.com/mail/
 
Reply With Quote
 
nelu
Guest
Posts: n/a
 
      10-28-2005

> > log_file = malloc(STRGSIZE);

>
> Always check the result of malloc(); if it fails, it will return a
> null pointer. Often the only thing you can do in response is to abort
> the program, but it's better than continuing blindly.


I've seen this in a lot of places and I've been wandering if it's smart
to write code like this:

log_file=malloc(STRGSIZE)

instead of:

log_file=(char *)malloc(STRGSIZE*sizeof(char))

even if it's faster to write
because it is both a portability problem and a lot of students don't
get the idea and do the exact same thing for types other than char?

 
Reply With Quote
 
VISHNU VARDHAN REDDY UNDYALA
Guest
Posts: n/a
 
      10-29-2005
thx a lot my friends
i got the ans
vishnu

 
Reply With Quote
 
Flash Gordon
Guest
Posts: n/a
 
      10-29-2005
nelu wrote:
>>> log_file = malloc(STRGSIZE);

>>
>>Always check the result of malloc(); if it fails, it will return a
>>null pointer. Often the only thing you can do in response is to abort
>>the program, but it's better than continuing blindly.

>
> I've seen this in a lot of places and I've been wandering if it's smart
> to write code like this:
>
> log_file=malloc(STRGSIZE)
>
> instead of:
>
> log_file=(char *)malloc(STRGSIZE*sizeof(char))
>
> even if it's faster to write
> because it is both a portability problem and a lot of students don't
> get the idea and do the exact same thing for types other than char?


No, the second option is far worse. You don't need the cast and
including it can hide a failure to include stdlib.h. If you want a more
generic form use:

ptr = malloc(N * sizeof *ptr);

Where N is the number of elements you want ptr to point to.
--
Flash Gordon
Living in interesting times.
Although my email address says spam, it is real and I read it.
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Problem problem problem :( Need Help Mike ASP General 2 05-11-2004 08:36 AM



Advertisments
 



1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57