Why this de-referencing does not work?

L

linq936

Hi,
My program crashes on the following code,

void lower(char* src, char* ret){
while ( src ) {
char c = tolower(*src);
*ret = c; <----------------------------- crash here
++ret;
++src;
}
return;
}

the calling of this function is like this,

char* s1 = "aBcDeFg";
char* s2 = "1234567";
lower(s1, s2);

Do you see what is wrong?
 
K

Karl Malbrain

Hi,
My program crashes on the following code,

void lower(char* src, char* ret){
while ( src ) {
char c = tolower(*src);
*ret = c; <----------------------------- crash here
++ret;
++src;
}
return;
}

You continue your loop until the pointer assumes a null value. You want to
continue until the character pointed to by the pointer is a null value.

karl m
 
R

Richard Heathfield

(e-mail address removed) said:
Hi,
My program crashes on the following code,

void lower(char* src, char* ret){
while ( src ) {

This should be:

while(*src) {

That's the first fix.
char c = tolower(*src);
*ret = c; <----------------------------- crash here
++ret;
++src;
}
return;
}

the calling of this function is like this,

char* s1 = "aBcDeFg";
char* s2 = "1234567";
lower(s1, s2);

Do you see what is wrong?

Yes, you're trying to write to memory you don't own. Here's the second
fix:

char *s1 = "aBcDeFg";
char s2[] = "1234567";
lower(s1, s2);
 
W

Walter Roberson

My program crashes on the following code,
void lower(char* src, char* ret){
while ( src ) {
char c = tolower(*src);
*ret = c; <----------------------------- crash here
++ret;
++src;
}
return;
}
the calling of this function is like this,
char* s1 = "aBcDeFg";
char* s2 = "1234567";
lower(s1, s2);
Do you see what is wrong?

char* s2 declares s2 to be a pointer to a character.

char* s2 = "1234567" goes further and initializes that pointer
to the address of a character array that has
'1' '2' '3' '4' '5' '6' '7' and a terminating 0 (not '0').

char* s2 = "1234567" is *not* defined as "allocate an area
of memory big enough to hold the literal string, copy the literal
string into that memory, and store the pointer to that memory
in s2". Instead, as far as the C language is concerned,
when you use the form char* s2 = "1234567", the area pointed
to is -required- to have static storage duration, and is
-permitted- to be in read-only memory.

It appears that in your implementation, literal strings are indeed
stored in read-only memory, so when you attempt to write to that
string in lower(), you get a fault. In other systems, there might
not be a fault -- but there might be the side effect that other
places that happened to also use "1234567" as literal
strings might suddenly start containing the string assigned in lower().
So Don't Do That!


What you can do instead is use

char s2[] = "1234567";

I know this -looks- like it should have exactly the same effect as

char* s2 = "1234567";

but when you use the char s2[] = literalstring form, C is defined
to allocate memory and copy the literal string in, leaving you
with an array of characters that you can modify. When you use
the char* s2 = literalstring form, you cannot modify the string.
 
K

Keith Thompson

My program crashes on the following code,

void lower(char* src, char* ret){
while ( src ) {
char c = tolower(*src);
*ret = c; <----------------------------- crash here
++ret;
++src;
}
return;
}

the calling of this function is like this,

char* s1 = "aBcDeFg";
char* s2 = "1234567";
lower(s1, s2);

Do you see what is wrong?

Yes.

The comp.lang.c FAQ is at <http://www.c-faq.com/>.
You've asked question 1.32.
 
L

Lew Pitcher

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,
My program crashes on the following code,

void lower(char* src, char* ret){
while ( src ) {
char c = tolower(*src);
*ret = c; <----------------------------- crash here
++ret;
++src;
}
return;
}

the calling of this function is like this,

char* s1 = "aBcDeFg";
char* s2 = "1234567";
lower(s1, s2);

Do you see what is wrong?

Yes

1) Your loop will terminate when src equals (char *)0 (the NULL pointer). You
don't want this; you want the loop to terminate when *src equals \0

2) If indeed you are calling your lower() function with the s1 and s2 you
presented, then you are doing it wrong. As s2 is a pointer to a fixed string
(/what/ does the standard call that?), alteration of the characters in that
string invokes (IIRC) undefined behaviour. Your operating system /may/ cause
the program to abend with the equivalent of a "trying to alter unalterable
memory" error. Instead, you want to call lower() with ret pointing to
alterable memory, something like...

{
void lower(char *, char *);
char *s1 = "This is a string", /* unalterable */
s2[60]; /* alterable */

lower(s1,s2);
}


3) In lower(), you don't length check the space pointed to by the ret
argument. What would happen if you called lower() with a ret that points to a
smaller area than the src points to? As in...
{
void lower(char *, char *);
char s1[] = "This is a long string",
s2[] = "Short";

lower(s1,s2);
}

HTH
- --
Lew Pitcher

Master Codewright & JOAT-in-training | Registered Linux User #112576
http://pitcher.digitalfreehold.ca/ | GPG public key available by request
- ---------- Slackware - Because I know what I'm doing. ------


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Armoured with GnuPG

iD8DBQFGX2WTagVFX4UWr64RAl4BAKCAwV+6kG6gOzkrt15dg6H4FVGbFACeJ/LL
/LUp15tkEqcp41XPHAof9+U=
=3L12
-----END PGP SIGNATURE-----
 
A

Army1987

Lew Pitcher said:
3) In lower(), you don't length check the space pointed to by the ret
argument. What would happen if you called lower() with a ret that points
to a
smaller area than the src points to? As in...
{
void lower(char *, char *);
char s1[] = "This is a long string",
s2[] = "Short";

lower(s1,s2);
}

AFAIK there is no way to check that from within lower(). You can only
prevent lower() from being called with such arguments.
 
L

Lew Pitcher

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Lew Pitcher said:
3) In lower(), you don't length check the space pointed to by the ret
argument. What would happen if you called lower() with a ret that points
to a
smaller area than the src points to? As in...
{
void lower(char *, char *);
char s1[] = "This is a long string",
s2[] = "Short";

lower(s1,s2);
}

AFAIK there is no way to check that from within lower(). You can only
prevent lower() from being called with such arguments.

Yup. You are correct. Sort of.

With the function prototype that the OP uses, there is no way that lower() can
perform any sort of check.

If the function prototype for lower() were changed a bit, the function could
perform a rudimentary check, as in
lower(char *source, char *target, size_t targetlength)
{
if (targetlength <= 0 strlen(source)) return;

or similar, then some small level of assurance could be established. It
becomes a little bit harder to shoot yourself in the foot with the OP's
lower() function.

Ideally, the function would take absolute pains to ensure that the target
string allocation was of a suitable size for the converted data. This /could/
be done by either
a) having the lower() function allocate the target string itself (returning
a char *target, and possibly obliging the caller to perform the free() ), or
b) having the lower() function replace in place the source string with the
target string,

In any case, my point is valid, and a solution is achievable.

- --
Lew Pitcher

Master Codewright & JOAT-in-training | Registered Linux User #112576
http://pitcher.digitalfreehold.ca/ | GPG public key available by request
- ---------- Slackware - Because I know what I'm doing. ------


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Armoured with GnuPG

iD8DBQFGYuXqagVFX4UWr64RAmfHAKDB0wmIoSF6CTAXeQPnDBq5MVGhFACgqEBg
2J92IlBywrY5nhv1C8Fa46g=
=DkMM
-----END PGP SIGNATURE-----
 
L

Lew Pitcher

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Lew Pitcher wrote:
... snip ...

size_t is unsigned. Such a value cannot be < 0.

Oops... sorry for the typo
I meant
if (targetlength <= strlen(source)) return;

I don't know how that stray zero got in there :-S

- --
Lew Pitcher

Master Codewright & JOAT-in-training | Registered Linux User #112576
http://pitcher.digitalfreehold.ca/ | GPG public key available by request
- ---------- Slackware - Because I know what I'm doing. ------


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Armoured with GnuPG

iD8DBQFGY3xeagVFX4UWr64RAihnAJ4hJeyzpmK5dcFLfUbVc5nFIAzNhgCg57rF
JX2Ed9Furqtex79WBgkBxsk=
=VfMy
-----END PGP SIGNATURE-----
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,772
Messages
2,569,591
Members
45,100
Latest member
MelodeeFaj
Top