Query on pointers

[ajinkya.coep]

What's wrong with this program? If you were to fix it, what would the
intended output be?

void swap(char *str, int index1, int index2) {
char tmp = str[index1];
str[index1] = str[index2];
str[index2] = tmp;
}

int main(int argc, char *argv[]) {
char *planet1;
char *planet2;

planet1 = (char *) malloc(7 * sizeof(char));
if (!planet1)
return 0;

snprintf(planet1, 7, "Jupiter");
planet2 = "Saturn";

swap(planet1, 0, 3);
swap(planet2, 3, 4);

printf("results: %s and %s\n", planet1, planet2);
return 0;
}

My interpretation : In turboc3 compiler(though it is a dead compiler
now) this problem is perfectly fine and we
get the o/p :

results: iupJter and Satrun

But in Bloodshed Dev C++ the code compiles fine but on
execution there is an error report generated.

swap(planet1,0,3) works fine but there is some problem
with swap(planet2,3,4)

Only when i allocate space for planet2 and perform
snprintf on planet2 i get the expected result.

Why do we need snprintf in this case....why does
planet2="Saturn" not work?

PSls send your comments and correct me if i am wrong...i am working
on this problem for 2 long days.

 

[=?ISO-8859-1?Q?Bj=F8rn_Augestad?=]

What's wrong with this program? If you were to fix it, what would the
intended output be?

void swap(char *str, int index1, int index2) {
char tmp = str[index1];
str[index1] = str[index2];
str[index2] = tmp;
}

int main(int argc, char *argv[]) {
char *planet1;
char *planet2;

planet1 = (char *) malloc(7 * sizeof(char));

"Jupiter" is 7 characters, so you must allocate 8. Why? You figure it out...

if (!planet1)
return 0;

snprintf(planet1, 7, "Jupiter");

Use strcpy().

planet2 = "Saturn";

Here planet2 points to a string literal, which in most cases is
non-modifiable.
..

swap(planet1, 0, 3);
swap(planet2, 3, 4);

And here you write to planet2. That's not a good idea, as you discovered.


HTH
Bjørn
[snip]

 

[mark_bluemel]

What's wrong with this program?

It's broken...

If you were to fix it, what would the intended output be?

I don't know. It's not my program.

void swap(char *str, int index1, int index2) {
char tmp = str[index1];
str[index1] = str[index2];
str[index2] = tmp;
}

int main(int argc, char *argv[]) {
char *planet1;
char *planet2;

planet1 = (char *) malloc(7 * sizeof(char));

I would suggest that you don't cast the result of malloc - use
<stdlib.h> - see FAQ http://c-faq.com/malloc/mallocnocast.html.

sizeof(char) is by definition, I believe, 1, so it served no purpose.

You've allocated 7 characters for planet1.

if (!planet1)
return 0;

snprintf(planet1, 7, "Jupiter");

"Jupiter" is not actually 7 characters long, is it? The result in the
space pointed to by planet1 will be "Jupite" (plus the obligatory
trailing '\0';

Why you felt the need to use snprintf() rather than strcpy(), or
strncpy(), I can't imagine.

planet2 = "Saturn";

planet2 points to data which may well be read-only...

swap(planet1, 0, 3);

This will swap the "J" and "i" in "Jupite".

swap(planet2, 3, 4);

On my system, as I'd expected, this crashes with segmentation
violation, as you try to modify data in read-only memory.

printf("results: %s and %s\n", planet1, planet2);
return 0;

}

My interpretation : In turboc3 compiler(though it is a dead compiler
now) this problem is perfectly fine and we
get the o/p :

results: iupJter and Satrun

Really? turboc3 is broken in that case, as the 1st planet cannot
legitimately be "Jupiter" ...

But in Bloodshed Dev C++ the code compiles fine but on
execution there is an error report generated.

swap(planet1,0,3) works fine but there is some problem
with swap(planet2,3,4)

Only when i allocate space for planet2 and perform
snprintf on planet2 i get the expected result.

Why do we need snprintf in this case....why does
planet2="Saturn" not work?

See my comments above on read-only memory.

 

[Martin Ambuhl]

What's wrong with this program? If you were to fix it, what would the
intended output be?

Whatever the redesigned program was written to output.

It is obviously wrong to omit
#include <stdlib.h>

void swap(char *str, int index1, int index2) {
char tmp = str[index1];
str[index1] = str[index2];
str[index2] = tmp;
}

int main(int argc, char *argv[]) {
char *planet1;
char *planet2;

planet1 = (char *) malloc(7 * sizeof(char));

The cast is unnecessay and bad programming practice.
sizeof(char) is by definition 1.
If you want no more flexibility than your code,
planet1 = malloc(7);
The magic number '7' is a bad idea, of course.

if (!planet1)
return 0;

snprintf(planet1, 7, "Jupiter");
planet2 = "Saturn";

Replace all of the above with
char planet1[] = "Jupiter";
char planet2[] = "Saturn";

swap(planet1, 0, 3);
swap(planet2, 3, 4);

You have just tried to modify a string literal. This is a very bad idea
and has unpredicatable consequences. One possible one is a program
crash. If you replace all of the code in main before the swap with the
two lines I suggested, this problem will not arise.

printf("results: %s and %s\n", planet1, planet2);
return 0;
}

My interpretation : In turboc3 compiler(though it is a dead compiler
now) this problem is perfectly fine and we

That TurboC allowed you to get away with this is just bad luck. It
would have been better for you if your program has crashed.

get the o/p :

results: iupJter and Satrun

But in Bloodshed Dev C++ the code compiles fine but on
execution there is an error report generated.

Because you foolishly try to modify a string literal.

swap(planet1,0,3) works fine but there is some problem
with swap(planet2,3,4)

No kidding.

Only when i allocate space for planet2 and perform
snprintf on planet2 i get the expected result.

Why do we need snprintf in this case....why does
planet2="Saturn" not work?

You don't need snprintf(). In addition to my suggested replacement
above, any of strcpy, memcpy, memmove, and sprintf could have been used.
There are other choices as well.
planet2 = "Saturn";
*does* work. It points planet2 to a string literal. It is you attempt
to modify the string literal that fails.

PSls send your comments and correct me if i am wrong...i am working
on this problem for 2 long days.

You could have just checked the FAQ articles on the difference between
pointers and arrays in minutes, or even (horrors) read your textbook,
preventing the waste of 2 long days.

 

[Kenneth Brody]

What's wrong with this program? If you were to fix it, what would the
intended output be?

void swap(char *str, int index1, int index2) {
char tmp = str[index1];
str[index1] = str[index2];
str[index2] = tmp;
}

int main(int argc, char *argv[]) {
char *planet1;
char *planet2;

planet1 = (char *) malloc(7 * sizeof(char));
if (!planet1)
return 0;

snprintf(planet1, 7, "Jupiter");
planet2 = "Saturn";

swap(planet1, 0, 3);
swap(planet2, 3, 4);

printf("results: %s and %s\n", planet1, planet2);
return 0;
}

Question: What should the final printf() do with planet1, which
was allocated 7 bytes, all of which were filled with something
other than '\0'?

My interpretation : In turboc3 compiler(though it is a dead compiler
now) this problem is perfectly fine and we
get the o/p :

results: iupJter and Satrun

You got (un)lucky in that it gave the results you expected, rather
than crashing.

But in Bloodshed Dev C++ the code compiles fine but on
execution there is an error report generated.

It found the errors that TurboC3 failed to crash on.

swap(planet1,0,3) works fine but there is some problem
with swap(planet2,3,4)

planet2 is a pointer to a string literal, which may be placed in
read-only memory.

Only when i allocate space for planet2 and perform
snprintf on planet2 i get the expected result.

By making that change, you have guaranteed that the memory it points
to is writable. (Assuming that malloc worked, of course.)

Why do we need snprintf in this case....why does
planet2="Saturn" not work?

See above. The string "Saturn" can be placed in read-only memory.

PSls send your comments and correct me if i am wrong...i am working
on this problem for 2 long days.

Who is "Pls"?

The ancient turboc3 compiler (16-bit real mode?) didn't place the
string literal into read-only memory. (There was no such thing in
the x86's "real mode" memory scheme, unless it was something like
ROM.) The Bloodshed Dev C++ complier probably runs in "protected
mode" on the x86 CPU, allowing it to place code and data in read-
only memory.

Also, your 7-byte buffer for the 8-byte string "Jupiter" (with the
'\0' terminator) "works" only by luck. The runtime library is
probably allocating more than 7 bytes, in order to handle proper
alignment, so you happen to have additional memory after your 7
bytes, and the 8th byte happens to be zero. Otherwise, the final
printf() wouldn't have stopped after the 7 characters "iupJter".
It would have kept going until it hit a '\0' or crashed.

--
+-------------------------+--------------------+-----------------------+
| Kenneth J. Brody | www.hvcomputer.com | #include |
| kenbrody/at\spamcop.net | www.fptech.com | <std_disclaimer.h> |
+-------------------------+--------------------+-----------------------+
Don't e-mail me at: <mailto:[email protected]>

 

[mark_bluemel]

Also, your 7-byte buffer for the 8-byte string "Jupiter" (with the
'\0' terminator) "works" only by luck. The runtime library is
probably allocating more than 7 bytes, in order to handle proper
alignment, so you happen to have additional memory after your 7
bytes, and the 8th byte happens to be zero. Otherwise, the final
printf() wouldn't have stopped after the 7 characters "iupJter".
It would have kept going until it hit a '\0' or crashed.

I think, indeed I'm fairly sure, you are mistaken.

The snprintf() call will move "Jupite" into the buffer allocated, and
then add the null character to make it a legal C string.

Either the original poster is mistaken about the output from turboc,
or it's broken.

 

[ak]

I think, indeed I'm fairly sure, you are mistaken.

The snprintf() call will move "Jupite" into the buffer allocated, and
then add the null character to make it a legal C string.

Either the original poster is mistaken about the output from turboc,
or it's broken.

Check it on any "decent" compiler snprintf() moves "Jupiter" and not
"Jupite" ....i think you are mistaken.

 

[Beej Jorgensen]

Check it on any "decent" compiler snprintf() moves "Jupiter" and not
"Jupite" ....i think you are mistaken.

#include <stdio.h>

int main(void)
{
char s[10];
snprintf(s, 7, "%s", "Jupiter");
printf("\"%s\"\n", s);
return 0;
}

$ cc --version
cc (GCC) 3.4.6

$ foo
"Jupite"

C99 7.19.6.5p2, The snprintf Function:
# output characters beyond the n-1st are discarded rather than being
# written to the array, and a null character is written at the end of
# the characters actually written into the array.

-Beej

 

[Bart van Ingen Schenau]

Check it on any "decent" compiler snprintf() moves "Jupiter" and not
"Jupite" ....i think you are mistaken.

Then you have a strange notion of 'decent compiler'.
Any *conforming* implementation, will truncate the string to "Jupite",
as that is what the standard requires for snprintf().

Bart v Ingen Schenau

 

[Ajinkya]

Then you have a strange notion of 'decent compiler'.
Any *conforming* implementation, will truncate the string to "Jupite",
as that is what the standard requires for snprintf().

Bart v Ingen Schenau

wont you call Bloodshed Dev C++ a "decent" compiler.......i know
ideally any compiler should not do it but all the compilers i have do
it("Jupiter")

 

[Bart van Ingen Schenau]

wont you call Bloodshed Dev C++ a "decent" compiler.......i know
ideally any compiler should not do it but all the compilers i have do
it("Jupiter")

I call it a non-conforming (C99) implementation.
Which does not surprise me, because to my knowledge, it does not claim
conformance to C99.

Whether it is a "decent" implementation, depends on how it copes with
things like this:

    char planet1[7];
    snprintf(planet1, 7, "Jupiter (The big red planet in our solar
system)");

Bart v Ingen Schenau

 

[Richard Heathfield]

Bart van Ingen Schenau said:

Whether it is a "decent" implementation, depends on how it copes with
things like this:

char planet1[7];
snprintf(planet1, 7, "Jupiter (The big red planet in our solar
system)");

This is rather like judging whether a Formula One car is "decent" by
smacking it into the wall at 200mph to see if the air-bag goes off.

 

[Kenneth Brody]

I think, indeed I'm fairly sure, you are mistaken.

The snprintf() call will move "Jupite" into the buffer allocated, and
then add the null character to make it a legal C string.

Either the original poster is mistaken about the output from turboc,
or it's broken.

Well, some further investigation shows that the FreeBSD man pages say:

The snprintf() and vsnprintf() functions will write at most size-1
of the characters printed into the output string (the size'th
character then gets the terminating `\0'); [...] The output is
always null-terminated.

It also says that snprintf() is C99.

My C90 implementation doesn't even have snprintf(). However, it does
have an _snprintf(), which says:

The _snprintf function formats and stores count or fewer characters
and values (including a terminating null character that is always
appended unless count is zero or the formatted string length is
greater than or equal to count characters) in buffer.

So, it appears that a C99 implementation would put "Jupite". However,
a pre-C99 implementation is not required to do so. Given the fact
that the OP said the output included the 'r' in "iupJter", my guess is
that it is not a C99 implementation, and is not nul-terminating the
string.

--
+-------------------------+--------------------+-----------------------+
| Kenneth J. Brody | www.hvcomputer.com | #include |
| kenbrody/at\spamcop.net | www.fptech.com | <std_disclaimer.h> |
+-------------------------+--------------------+-----------------------+
Don't e-mail me at: <mailto:[email protected]>

 

[Old Wolf]

What's wrong with this program? If you were to fix it, what would the
intended output be?

Slightly strange question, as who can know the intended output
except for the person who wrote it?

planet1 = (char *) malloc(7 * sizeof(char));
snprintf(planet1, 7, "Jupiter");

Something nobody has mentioned yet: you failed to write:
#include <stdlib.h>
#include <stdio.h>

so the above lines cause undefined behaviour.

(Sometimes it can be assumed that these includes were omitted
for brevity, but I don't feel confident the OP knows that these lines
are required).

 

[Old Wolf]

I don't know. It's not my program.



Why you felt the need to use snprintf() rather than strcpy(), or
strncpy(), I can't imagine.

I'm surprised that multiple people made this comment. strcpy
and strncpy would both cause the program to have undefined
behaviour (assuming appropriate headers are included in
all cases, that is).

As it is, snprintf will not cause UB. Conceivably the result is
not the inteded one -- but as you pointed out, we can't know
what the intended result even was.

 

[Richard Heathfield]

Old Wolf said:

I'm surprised that multiple people made this comment. strcpy
and strncpy would both cause the program to have undefined
behaviour (assuming appropriate headers are included in
all cases, that is).

Never mind strcpy, strncpy, and snprintf - why he felt the need to use
malloc is beyond me. Simple arrays of char would have done the trick.