tguclu <tu**********@g mail.comwrites:
[...]
Why passing string literal crashes the program?
I've completed and made some tryings on his code.
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
char* lower(char* str)
{
char* p = str;
while ( *str )
{
*str = tolower(*str); //<=====crash!
str++;
}
return p;
}
A stylistic note: you copy the original value of the parameter 'str'
to 'p', then modify 'str' in your loop. There's nothing wrong with
that, but I'd probably do it the other way around: maintain 'str' with
its original value, and modify the copy. It's not a big deal, I just
find it a bit clearer that way.
int main()
That's ok, but it's better to be more explicit:
int main(void)
{
char *dd = (char *)malloc(sizeof (char)*2);
Don't cast the result of malloc. See questions 7.7, 7.7b, and 7.7c in
the FAQ, <http://www.c-faq.com/>. In fact, *most* casts are
unnecessary.
sizeof(char) is 1 by definition. If you know you're always going to
be allocating characters, you can write the above as:
char *dd = malloc(2);
If there's some possibility that you might change dd to some other
pointer type in the future, the recommended idiom is:
char *dd = malloc(2 * sizeof *dd);
(By applying sizeof to *dd, you automatically get the right type even
if you change the declaration).
malloc can fail; it indicates failure by returning a null pointer
value. Always check for this. (It's probably ok to leave out the
check in a small sample program like this one, but in that case add a
comment saying that you've left it out.)
strcpy(dd,"DD") ;
As somebody else pointed out, this copies 3 bytes ('D', 'D', '\0')
into an array of 2 elements (assuming malloc succeeded).
If I wanted to make sure the code is flexible in the face of future
changes, I might do this:
#define THE_STRING "DD"
char *dd = malloc(sizeof THE_STRING);
/* check whether malloc failed */
strcpy(dd, THE_STRING);
or perhaps:
#define THE_STRING "DD"
char *dd = malloc(strlen(T HE_STRING) + 1);
/* check whether malloc failed */
strcpy(dd, THE_STRING);
I'm not suggesting that this kind of thing is necessary for a small
sample program like this one, but it's something to keep in mind for
future programs.
puts(lower(dd)) ; //crashes, if changed with puts(lower("DD" ));
return 0;
}
[...]
--
Keith Thompson (The_Other_Keit h)
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."
-- Antony Jay and Jonathan Lynn, "Yes Minister"
