This forum is a read-only archive of the Gammon Software forum (2000–2026). No new posts can be made. Search the archive.

Converting from GCC to G++

Posted by Atrox on Fri 19 Mar 2010 01:35 AM — 30 posts, 99,080 views.

Atrox USA #0
So, I recently started converting my heavily modified SmaugFUSS 1.6 from compiling with GCC to G++. I used the most recent SmaugFUSS 1.9 as a reference, and FINALLY got it all compiling and linking properly today. Now, the problem is... the interpreter is crashing when trying to allocate memory for a temporary pointer to convert input from a normal character pointer to a constant.

Just as in the regular SmaugFUSS 1.9, I'm using this function for that:
(225)  void interpret( CHAR_DATA * ch, const char* argument)
(226)  {
(227)      char *temp = strdup(argument);
(228)      interpret(ch, temp);
(229)      free(temp);
(230)  }


In GDB, this is what I get:
#0  0xb7b6086c in _int_malloc () from /lib/libc.so.6
#1  0xb7b6250f in calloc () from /lib/libc.so.6
#2  0x080f47a0 in str_dup (str=0x86531f8 "who") at db.c:3447
#3  0x0812d775 in interpret (ch=0x83a73e8, argument=0x86531f8 "who") at interp.c:227
#4  0x0812d789 in interpret (ch=0x83a73e8, argument=0x86531e8 "who") at interp.c:228
#5  0x0812d789 in interpret (ch=0x83a73e8, argument=0x86531d8 "who") at interp.c:228
Nick Gammon Australia Forum Administrator #1
Atrox said:

Just as in the regular SmaugFUSS 1.9, I'm using this function for that:
(225)  void interpret( CHAR_DATA * ch, const char* argument)
(226)  {
(227)      char *temp = strdup(argument);
(228)      interpret(ch, temp);
(229)      free(temp);
(230)  }



The function interpret is calling itself. Is that intentional?
Atrox USA #2
It's an overloaded function, the first one that's called is through the main loop using a constant character pointer for arg2, and then it passes it to another interpret function that takes a regular character pointer.


// New overloaded function to accept const char *'s
void interpret( CHAR_DATA * ch, const char* argument)
{
    char *temp = strdup(argument);
    interpret(ch, temp);
    free(temp);
}

// Original Smaug implementation
void interpret( CHAR_DATA * ch, char *argument )
{
Amended on Fri 19 Mar 2010 02:37 AM by Atrox
Hokken USA #3
you really can't overload a function based on the const-ness of a pointed to type in C++ - it's too ambiguous. that's why your first interpret() function is calling itself recursively until you either overflow your stack or run out of heap. why are you overloading the function in the first place if all you are doing is calling the original one? if you really need to have two different functions then you have to either name them differently or use unambiguously different type signatures.
Nick Gammon Australia Forum Administrator #4
Well I thought you couldn't have function declarations that only differ by const-ness, but the g++ compiler seems to accept that.

This reproduces your problem:


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

// New overloaded function to accept const char *'s
void interpret(char * ch, const char* argument)
  {
  char *temp = strdup(argument);
  interpret(ch, temp);
  free(temp);
  }

// Original Smaug implementation
void interpret( char * ch, char *argument )
  {
  printf ("argument is %s\n", argument);
  }

int main (void)
  {
  interpret ("nick", "who");
  return 0;
  }


Running it:


g++ -g3 test.cpp

$ ./a.out
Segmentation fault

gdb ./a.out
(gdb) run


#0  0xb7e8494c in ?? () from /lib/tls/i686/cmov/libc.so.6
#1  0xb7e868c5 in malloc () from /lib/tls/i686/cmov/libc.so.6
#2  0xb7e8a060 in strdup () from /lib/tls/i686/cmov/libc.so.6
#3  0x08048540 in interpret (ch=0x8048670 "nick", argument=0xa021b98 "who")
    at test.cpp:9
#4  0x08048555 in interpret (ch=0x8048670 "nick", argument=0xa021b88 "who")
    at test.cpp:10
#5  0x08048555 in interpret (ch=0x8048670 "nick", argument=0xa021b78 "who")
    at test.cpp:10
#6  0x08048555 in interpret (ch=0x8048670 "nick", argument=0xa021b68 "who")
    at test.cpp:10
#7  0x08048555 in interpret (ch=0x8048670 "nick", argument=0xa021b58 "who")
    at test.cpp:10
#8  0x08048555 in interpret (ch=0x8048670 "nick", argument=0xa021b48 "who")
    at test.cpp:10


Clearly it is recursing, as your own results show:


#0  0xb7b6086c in _int_malloc () from /lib/libc.so.6
#1  0xb7b6250f in calloc () from /lib/libc.so.6
#2  0x080f47a0 in str_dup (str=0x86531f8 "who") at db.c:3447
#3  0x0812d775 in interpret (ch=0x83a73e8, argument=0x86531f8 "who") at interp.c:227
#4  0x0812d789 in interpret (ch=0x83a73e8, argument=0x86531e8 "who") at interp.c:228
#5  0x0812d789 in interpret (ch=0x83a73e8, argument=0x86531d8 "who") at interp.c:228


However the solution is simple enough, forward-declare the function:


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

void interpret( char * ch, char *argument );

// New overloaded function to accept const char *'s
void interpret(char * ch, const char* argument)
  {
  char *temp = strdup(argument);
  interpret(ch, temp);
  free(temp);
  }

// Original Smaug implementation
void interpret( char * ch, char *argument )
  {
  printf ("argument is %s\n", argument);
  }

int main (void)
  {
  interpret ("nick", "who");
  return 0;
  }


Now that runs OK. However I am inclined to agree with Hokken that it is a bit dangerous.
Amended on Fri 19 Mar 2010 03:21 AM by Nick Gammon
Twisol USA #5
As I see it, there are two possibilities here.

(1) The function doesn't modify the string. If it does, the mutation is an unimportant side-effect.

(2) The function does modify the string, and the mutated string is part of the result set (C#'s "out" parameter keyword says it better).

If 1, you should just modify the original function to take a const char*. A char* can be casted to a const char* implicitly, so this is no trouble to the user. Then, if the function modifies the string (but it's not an actual "result" of the function, just a mere side-effect)) just put your strdup() code into the function itself.

If 2, you probably shouldn't be wrapping it with strdup, because you're trimming data that might be needed elsewhere. Use strdup in those locations rather than in the function.
Atrox USA #6
I still don't get why it works just fine in SmaugFUSS 1.9, but not here. It's already forward declared, in mud.h, but I changed the original Smaug function to interpret2, and prototyped it above the new function, and now it works fine. Except that in converting my code to compile with G++ I have broken my dynamic linker, so now I get to come home after work tomorrow and try to fix that. :(

Thanks a lot for the help.
David Haley USA #7
Quote:
you really can't overload a function based on the const-ness of a pointed to type in C++ - it's too ambiguous.

Sure you can; you just have to be sure that both prototypes are available whenever you use it.

Quote:
why are you overloading the function in the first place if all you are doing is calling the original one?

Because it's very convenient to be able to just call 'interpret' without worrying about the constness. The real implementation expects a modifiable string. But you occasionally want to interpret a constant string. So, you wrap the real implementation with a version that creates a modifiable string before disposing of it.


Twisol, your cases don't actually work in general. You assume that if it modifies it, those modifications are necessarily uninteresting. I'm not sure why you're making that assumption. It's perfectly reasonable to care about the modifications in some cases, and not care in others. If you have a const char* and actually need modifications, then you do the str dup yourself. But there's no point creating pain if you don't need the modifications most of the time.

That said, this is probably legacy, because the do_fun's have all been converted to take const char* arguments. That was the original reason behind all of this, IIRC -- if interp was const char*, it would have to be str_dup'ing internally anyhow because of what the commands were doing.
Twisol USA #8
David Haley said:
Twisol, your cases don't actually work in general. You assume that if it modifies it, those modifications are necessarily uninteresting. I'm not sure why you're making that assumption. It's perfectly reasonable to care about the modifications in some cases, and not care in others.

I only assume that in case 1, really. Case 2 assumes that the modifications are interesting. Basically, just use const char* if there are no modifications, or the modifications are internal; use char* if there are modifications and they're "interesting". But in neither case should you need to create an overloaded function just to strdup, I'd think.

David Haley said:
If you have a const char* and actually need modifications, then you do the str dup yourself.

==
Twisol said:
If 2, you probably shouldn't be wrapping it with strdup, because you're trimming data that might be needed elsewhere. Use strdup in those locations rather than in the function.
Amended on Fri 19 Mar 2010 05:34 AM by Twisol
Nick Gammon Australia Forum Administrator #9
Atrox said:

I still don't get why it works just fine in SmaugFUSS 1.9, but not here. It's already forward declared, in mud.h, ...


You didn't forward declare the non-const one, or you wouldn't have the crash.
David Haley USA #10
Twisol, your cases are:

1. No modifications, or modifications uninteresting
2. Modifications are always interesting

You're leaving out the case I pointed out: modifications are only sometimes interesting, and you don't want to have to go through any hassle when you don't care about them.

Using your suggestions, if you force the function to take a char* on the assumption that the modifications are always interesting, then callers who don't care must do extra work. A simple overloaded wrapper fixes this completely.

You also incorrectly equate my suggestion and your case 2. My suggestion is that you overload the function so that it's easy to not care, but in those cases where you do care, and all you have is a const char*, only then must you do any extra work.

In other words, my suggestion minimizes work necessary in all cases.

There are four cases:
1. you care and you have a char*, and everything works
2. you don't care and you have a char*, and everything works
3. you care and you have a const char*, you need to do some work
4. you don't care and you have a const char*, and everything works

Your suggestion, however, needlessly forces extra work in case #4.
Twisol USA #11
David Haley said:

Twisol, your cases are:

1. No modifications, or modifications uninteresting
2. Modifications are always interesting

You're leaving out the case I pointed out: modifications are only sometimes interesting, and you don't want to have to go through any hassle when you don't care about them.

Using your suggestions, if you force the function to take a char* on the assumption that the modifications are always interesting, then callers who don't care must do extra work. A simple overloaded wrapper fixes this completely.

You also incorrectly equate my suggestion and your case 2. My suggestion is that you overload the function so that it's easy to not care, but in those cases where you do care, and all you have is a const char*, only then must you do any extra work.

In other words, my suggestion minimizes work necessary in all cases.

There are four cases:
1. you care and you have a char*, and everything works
2. you don't care and you have a char*, and everything works
3. you care and you have a const char*, you need to do some work
4. you don't care and you have a const char*, and everything works

Your suggestion, however, needlessly forces extra work in case #4.


My definition of "interesting" in this case is more like "externally visible". If it modifies the source string at all, it should be "interesting". Correct me if I'm wrong, but your case 2 leaves this fact out, meaning the source string might be modified and you might not know about it.

Your cases are in regards to the user; mine are more in regards to the function itself.

1. If the function modifies the string and it should be publically visible, use char*.
2. If the function modifies the string and it shouldn't be publically visible, use const char*, and use strdup() internally.
3. If the function doesn't modify the string, use const char*.

Looking at how the user would use them:

1a. If you want the modifications, everything works.
1b. If you don't want the modifications, strdup.
2. Everything works.
3. Everything works.

The only work the user has to do is in the case of 1a, where you have to prevent the function from modifying a string that you want to preserve. This is logical.
David Haley USA #12
Function APIs are necessarily from the user's perspective. You don't design APIs from an implementer's perspective but from a user's, unless there are rather stringent implementation details to worry about, typically having to do with serious performance hits. Your proposed API creates needless work in some cases, while never reducing the amount of work. I'm not sure how this could possibly be a good thing. :-)

You continue to assume, by the way, that either changes should always or never be externally visible. You leave out the case where you sometimes care and sometimes don't.

The only possible argument against this, really, is that you need to be aware that the function can make some modifications that you might care about. This would need to be made clear in the API documentation.

In the end of the day, it is as simple as optimizing for the common case. If the common case is that you care about the modifications, then it might make sense to force extra work only to save the user from reading documentation. If the common case is that you don't care about the modifications, it is rather unambiguously better to save the user from needing to care about them. If the common case is split, then prefer the solution that creates the least amount of work for users.
Amended on Fri 19 Mar 2010 06:10 AM by David Haley
Twisol USA #13
David Haley said:

Function APIs are necessarily from the user's perspective. You don't design APIs from an implementer's perspective but from a user's, unless there are rather stringent implementation details to worry about, typically having to do with serious performance hits. Your proposed API creates needless work in some cases, while never reducing the amount of work. I'm not sure how this could possibly be a good thing. :-)

You continue to assume, by the way, that either changes should always or never be externally visible. You leave out the case where you sometimes care and sometimes don't.

The only possible argument against this, really, is that you need to be aware that the function can make some modifications that you might care about. This would need to be made clear in the API documentation.

In the end of the day, it is as simple as optimizing for the common case. If the common case is that you care about the modifications, then it might make sense to force extra work only to save the user from reading documentation. If the common case is that you don't care about the modifications, it is rather unambiguously better to save the user from needing to care about them. If the common case is split, then prefer the solution that creates the least amount of work for users.


Well, I'm also trying to keep in mind the actual function involved here, too. And when you say you "sometimes" care, how does the function deal with that? My preferred approach in that case would be something like this:

void interpret( CHAR_DATA * ch, const char* argument, char* out)


And you set 'out' to NULL when you don't care. A "sometimes" case, when handled without the 'out' parameter above, is not explicit enough for me. But considering the function at hand, it didn't seem like going that far was required. Maybe I was wrong.


Also, you should know me well enough by now to know that I usually go too far (in the eyes of some) in the name of user-friendliness, i.e. PPI. ;)
Amended on Fri 19 Mar 2010 06:28 AM by Twisol
David Haley USA #14
The 'out' parameter is another way to deal with this, but it (as usual) depends on what exactly is happening here. Sometimes it's more convenient to have the input operated on directly for whatever reason. For instance, it might be a more complicated data structure than just a char*. Also, it means more stuff to keep track of on the caller's end when they want to get the modifications; now they have to make the call and prepare the target buffer for the modifications. Like I said, it really depends on what the common case is.

All I was trying to say here is that there are reasonable cases besides "always care" and "never care", and so it can make sense to have this form of overloading. (Sometimes, the implementations might even be different, and the overloaded version wouldn't wrap it with a duplication but do some other computation that doesn't involve the side effect.) So the answer isn't a clear-cut "make it either const or non const but not both". Even in the case of the nullable out parameter, we see that this is happening (although out parameters, especially for some types in C/C++, have a whole new slew of issues of their own...).
Twisol USA #15
David Haley said:
The 'out' parameter is another way to deal with this, but it (as usual) depends on what exactly is happening here. Sometimes it's more convenient to have the input operated on directly for whatever reason. For instance, it might be a more complicated data structure than just a char*. Also, it means more stuff to keep track of on the caller's end when they want to get the modifications; now they have to make the call and prepare the target buffer for the modifications. Like I said, it really depends on what the common case is.

It's also completely possible to call it with the same variable as both the second and third argument, i.e. interpret(ch, command, command)

David Haley said:
All I was trying to say here is that there are reasonable cases besides "always care" and "never care", and so it can make sense to have this form of overloading. (Sometimes, the implementations might even be different, and the overloaded version wouldn't wrap it with a duplication but do some other computation that doesn't involve the side effect.) So the answer isn't a clear-cut "make it either const or non const but not both". Even in the case of the nullable out parameter, we see that this is happening (although out parameters, especially for some types in C/C++, have a whole new slew of issues of their own...).

Of course. Again, I'm just trying to keep the original issue in mind here.
David Haley USA #16
Twisol said:
It's also completely possible to call it with the same variable as both the second and third argument, i.e. interpret(ch, command, command)

Most of the time, that is in fact very dangerous. It's like working on input as you're pulling it out from under yourself. Most functions with in/out parameters require that they point to different things, or else much confusion results.
Twisol USA #17
David Haley said:

Twisol said:
It's also completely possible to call it with the same variable as both the second and third argument, i.e. interpret(ch, command, command)

Most of the time, that is in fact very dangerous. It's like working on input as you're pulling it out from under yourself. Most functions with in/out parameters require that they point to different things, or else much confusion results.

Fair point. My point was just that it would be basically exactly the same thing as an interpret() with only a char*.
David Haley USA #18
I guess I don't see how it's the same thing, because you're likely to have to duplicate it so that the program won't be overwriting the buffer it's reading from.

If your function is foo(const char* in, char* out), you most likely wouldn't want to pass the same char* to both, unless the function explicitly says that it is safe to do so. (This might be the case for a function that converts a string's case, for instance, so it only writes over input positions that it has already read.)

A color converter, for example, shows quite clearly how disastrous it can be to pass in a single string, because you'll be expanding color codes into the input you haven't seen yet.
Twisol USA #19
Put it this way... A "widening" type conversion, like conversion from a char to a long, is valid and safe. The reverse is not true, as you may be losing data in the higher bits. This is the same thing going on here; I'm widening from the original foo(char*) to foo(const char*, char*), whereas you're looking at it in the reverse direction. Assuming that both versions of the function still do the same thing, it should be legal to call foo()/2 with the same argument twice, which effectively degenerates to the original foo()/1.
David Haley USA #20
I don't understand the notation you're using, sorry. :)

I think what you're saying is that if the single-parameter version was safe, then it must not be overwriting its input anyhow. Well, that's not necessarily the case, either: it might be duplicating the input stream internally as a convenience to the caller, so that they pass in some input, and it comes back modified, without needing to worry about creating a buffer themselves.

Basically, my point here again is pretty simple: if a function has in and out parameters, you should be wary of using the same memory for both, unless the documentation explicitly allows it.
Twisol USA #21
David Haley said:
I don't understand the notation you're using, sorry. :)

Arity: I blatantly stole it from Erlang. foo()/2 means the version of foo() with 2 arguments. (Strictly speaking, I have the () in the wrong place, but I'm just using it descriptively)

David Haley said:
I think what you're saying is that if the single-parameter version was safe, then it must not be overwriting its input anyhow. Well, that's not necessarily the case, either: it might be duplicating the input stream internally as a convenience to the caller, so that they pass in some input, and it comes back modified, without needing to worry about creating a buffer themselves.

I was assuming that foo()/1 does modify its input string, otherwise I'd make it a const char*. If foo()/1 is safe, and it modifies its input string, then foo()/2 should be safe, assuming you replace the relevant variable references with 'out' instead. But you shouldn't have to change the logic, at all.

David Haley said:
Basically, my point here again is pretty simple: if a function has in and out parameters, you should be wary of using the same memory for both, unless the documentation explicitly allows it.

In general I agree. I still believe that a "widening" conversion gives you an implicit guarantee, at least until you actually change the logic in foo()/2.
David Haley USA #22
Twisol said:
I was assuming that foo()/1 does modify its input string, otherwise I'd make it a const char*. If foo()/1 is safe, and it modifies its input string, then foo()/2 should be safe, assuming you replace the relevant variable references with 'out' instead. But you shouldn't have to change the logic, at all.

No, I disagree with this.

Assume that foo(X*) is modifying its input string by first copying it, and then using the copy as input, and writing output back to the parameter. Also assume that foo(const X* in, X* out) assumes (rather reasonably) that the input and output buffers are different, and therefore does not need to copy its input. Therefore, it will be writing into the output even though it is squashing its input.

In other words, in the first case it might use the parameter memory as both input and output, and does this safely because it copies the input stream before operating on the parameter's memory.

So: it's not safe to assume that just because things work with the single-parameter version, they'll necessarily work the same way with the two-parameter version.
Twisol USA #23
David Haley said:
Also assume that foo(const X* in, X* out) assumes (rather reasonably) that the input and output buffers are different, and therefore does not need to copy its input.

There, right there, you are changing the logic of the original function, and breaking the guarantee I postulated.
Amended on Fri 19 Mar 2010 07:42 PM by Twisol
David Haley USA #24
What guarantee exactly am I breaking?

You can modify your input, without it being safe to have a version that reads and writes the same memory location.

You might be saying that you want the two-argument version to do the exact same thing as the one-argument version internally, just write to the second param. It would therefore still be duplicating the input parameter internally, because it's doing the exact same thing. If this is what you are saying, then ok, but this is kind of silly (it defeats the whole point of having a const parameter!) and regardless is not an assumption that can be made generally.

You can't assume, when looking at things in general, that just because it's safe to call the one-parameter version, it's also safe to call the two-parameter version with the same buffer unless special precautions have been taken (and the API documentation would presumably make this clear).

Obviously we can contrive any number of cases in which this would be safe, but I'm trying to make statements about general programming practices, not an extremely specific case. What got us into this tangent were comments made about constness and overloading in general, not on some specific case we make up. See in particular my first comment in this thread.

And even if you want to get into specifics, namely the SMAUG 'interpret' function, my previous comments argue that you want to make life as convenient as possible for the caller, and that your suggestions reduce convenience for the caller because they force the creation of a writable buffer, which is annoying when all you have is a const buffer: you need to do the duplication yourself. By contrast, if you have an overloaded version that hides that from you, you don't need to worry about it.

I guess I'm not sure what you're trying to argue. :-)
Twisol USA #25
David Haley said:
I guess I'm not sure what you're trying to argue. :-)

Me either, anymore. =/ I guess I just enjoy a good discussion.

I was originally trying to show how you probably don't need an overloaded function at all. and assuming that either the modifications were interesting or they weren't. Since the function returns void, the parameter is the only way to get any data back out of the function. So if it's interesting, use char*; if it's not interesting, use const char* and strdup(). In the first case, yes, I'm assuming the general case is that you want the interesting modifications. If you don't, well, I guess that's what the original post's overload was for. But that's far too implicit for my tastes, and I don't think overloading is a good fit for that.
Nick Gammon Australia Forum Administrator #26
Twisol said:

Since the function returns void, the parameter is the only way to get any data back out of the function.


No, the function has side-effects. So for example interpret ("who") shows a who list. The function interpret does not return data, per se.
Twisol USA #27
Nick Gammon said:

Twisol said:

Since the function returns void, the parameter is the only way to get any data back out of the function.


No, the function has side-effects. So for example interpret ("who") shows a who list. The function interpret does not return data, per se.


I meant that there's nothing that the function does that the calling code sees straight away. Of course it has side effects, that's the point of the function. ;) But the side-effected data is sent to the user over a socket, not back out of the interpret function, correct?
Amended on Sat 20 Mar 2010 01:51 AM by Twisol
Nick Gammon Australia Forum Administrator #28
Twisol said:

I meant that there's nothing that the function does that the calling code sees straight away. Of course it has side effects, that's the point of the function. ;) But the side-effected data is sent to the user over a socket, not back out of the interpret function, correct?


Yes, in this case.

However some functions (like math.floor for example) only work on the provided data, and return a result. Thus (in C at least) if they need to modify lots of things, you need to provide them with non-const arguments which are not simply call-by-value.

I am making the point that for some functions you may not *need* to get data back out of them, if they operate only by causing side-effects.
Twisol USA #29
Nick Gammon said:
I am making the point that for some functions you may not *need* to get data back out of them, if they operate only by causing side-effects.


That, by definition, is getting data back out of them, at least in my book. It's what I meant earlier about C#'s "out" parameter modifier.