C Strings not returning from a function

[pkirk25]

I've made a small program to demonstrate one problem I'm having fixing
strings in C.

I need to be able to remove HTML mark-ups from text lines.

I create my variable, pass it to my function, verify that the data has
been passed correctly and then cannot get the data back!

If I change the return to some random string literal, it comes back
fine.

All advice appreciated.

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

char * FixString(char *strIn)
{
char *strMsg = strIn;
char strTmp[40] = "";
char *strReturn = &strTmp;

int i = 0;
int j = strlen(strMsg);


for (i = 0; i <= j; ++i)
{
/* removing the cleanup code for brevity on usenet */
strTmp = strMsg;
}

printf("%s\n", strReturn);
/* Prints out the string perfectly */

return strReturn; /* change to return "All clear" and it works fine
*/
}

int main(void)
{
char *strOut = "This is a line from a web page <br>";
char *strBack = FixString(strOut);

printf("%s\n", strBack); /* Prints garbage - why? */

return 0;
}

 

[Ian Collins]

I've made a small program to demonstrate one problem I'm having fixing
strings in C.

I need to be able to remove HTML mark-ups from text lines.

I create my variable, pass it to my function, verify that the data has
been passed correctly and then cannot get the data back!

If I change the return to some random string literal, it comes back
fine.

All advice appreciated.

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

char * FixString(char *strIn)
{
char *strMsg = strIn;
char strTmp[40] = "";
char *strReturn = &strTmp;

Didn't your compiler give you a helpful diagnostic here? If not, turn
up its warning level or use a better compiler.

int i = 0;
int j = strlen(strMsg);


for (i = 0; i <= j; ++i)
{
/* removing the cleanup code for brevity on usenet */
strTmp = strMsg;
}

printf("%s\n", strReturn);
/* Prints out the string perfectly */

return strReturn; /* change to return "All clear" and it works fine

You are attempting to return a pointer to a local variable, don't do
this. Either pass in the output string, or use a dynamic buffer
allocated by the function and return this.

 

[pkirk25]

[snip]
My complier is Visual C++ 6 wcih I know is old but it is what was
available.

I genuinely think my problems are dwon to struggling with indirection
and pointers moret han the compiler, tempting tho it is to blame
Microsoft.

You are attempting to return a pointer to a local variable, don't do
this. Either pass in the output string, or use a dynamic buffer
allocated by the function and return this.

Ian, if i retrun the string array strTmp that *strReturn points to, I
get the same mess.

Can you explain what you mean by "pass in the output string"? What
would I change in my code to do that? Sorry to ask to be spoon fed.

 

[Ian Collins]

[snip]
My complier is Visual C++ 6 wcih I know is old but it is what was
available.

I genuinely think my problems are dwon to struggling with indirection
and pointers moret han the compiler, tempting tho it is to blame
Microsoft.

A C++ compiler should refuse to compile the line you snipped;

char *strReturn = &strTmp;

Here you are attempting to initialise a char* with a char**, either use

char *strReturn = strTmp;

or

char *strReturn = &strTmp[0];

Ian, if i retrun the string array strTmp that *strReturn points to, I
get the same mess.

Can you explain what you mean by "pass in the output string"? What
would I change in my code to do that? Sorry to ask to be spoon fed.

Add an extra parameter:

char* FixString( const char* strIn, char* strOut )

Note the change of strIn to const, you aren't changing it in your
function. Use strOut where you use strTmp.

You can then call it with:

const char *strOut = "This is a line from a web page <br>"

char *backBuff = char[someSize];

char *strBack = FixString(strOut, backBuff);

 

[Keith Thompson]

[snip]
My complier is Visual C++ 6 wcih I know is old but it is what was
available.

I genuinely think my problems are dwon to struggling with indirection
and pointers moret han the compiler, tempting tho it is to blame
Microsoft.

A C++ compiler should refuse to compile the line you snipped;

char *strReturn = &strTmp;

Here you are attempting to initialise a char* with a char**, either use

char *strReturn = strTmp;

or

char *strReturn = &strTmp[0];

A C compiler should refuse to compile it.

Actually, that's not quite true. Attempting to initialize a char*
with a char** is a constraint violation. A conforming C compiler must
issue a diagnostic. Once it's done so, it's not required either to
accept or to reject the translation unit.

<OT>I *think* the rules are similar in C++, but I'm too lazy to search
through the C++ standard for the corresponding wording. In any case,
I *think* that Visual C++ can be used as a C compiler.</OT>

 

[pkirk25]

Code almsot half the size and works perfectly.

Thanks!

BTW, the complier warns that malloc returns int and that my *backBuff
is not one in the line:

char *backBuff = malloc(i);

If I fix that, no more VC++ warnings.


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

char * FixString(char *strIn, char *strOut)
{
int i = 0;
int j = strlen(strIn);


for (i = 0; i <= j; ++i)
{
/* removing the cleanup code for brevity on usenet */
strOut = strIn;
}

printf("%s\n", strOut);


return strOut;
}

int main(void)
{
char *strOut = "This is a line from a web page <br>";
int i = strlen(strOut);
char *backBuff = malloc(i);
char *strBack = FixString(strOut, backBuff);

printf("%s\n", strBack); /* Prints out the string perfectly */

return 0;
}

 

[Ian Collins]

Code almsot half the size and works perfectly.

Please keep some context in your replies.

Thanks!

BTW, the complier warns that malloc returns int and that my *backBuff
is not one in the line:

That's because you haven't included the required header, <stdlib.h>

 

[pkirk25]

"0 error(s), 0 warning(s)"

Well, that means i can go to bed!

Thanks and good night.

 

[Ian Collins]

"0 error(s), 0 warning(s)"

Well, that means i can go to bed!

Thanks and good night.

Didn't you see my comment about context?

 

[Ian Collins]

A C compiler should refuse to compile it.

Actually, that's not quite true. Attempting to initialize a char*
with a char** is a constraint violation. A conforming C compiler must
issue a diagnostic. Once it's done so, it's not required either to
accept or to reject the translation unit.

<OT>I *think* the rules are similar in C++, but I'm too lazy to search
through the C++ standard for the corresponding wording. In any case,
I *think* that Visual C++ can be used as a C compiler.</OT>

<OT>In C++, you can't assign or initialise with incompatible types, it's
an error.</OT>

I can't see why C doesn't enforce the same rules, if you want to do an
incompatible assignment, you can force the issue with a cast.

 

[Ian Collins]

[snip]
My complier is Visual C++ 6 wcih I know is old but it is what was
available.

I genuinely think my problems are dwon to struggling with indirection
and pointers moret han the compiler, tempting tho it is to blame
Microsoft.

A C++ compiler should refuse to compile the line you snipped;

char *strReturn = &strTmp;

Here you are attempting to initialise a char* with a char**, either use

Oops, I should have written "attempting to initialise a char* with a
pointer to an array of char".

 

[CBFalconer]

.... snip ...

I create my variable, pass it to my function, verify that the data
has been passed correctly and then cannot get the data back!

If I change the return to some random string literal, it comes
back fine.

All advice appreciated.

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

char * FixString(char *strIn)
{
char *strMsg = strIn;
char strTmp[40] = "";
char *strReturn = &strTmp;
.... snip ...

return strReturn; /* change to return "All clear" and it works fine
*/

You are returning the address of local storage, which storage no
longer exists after the function returns. Change the prototype to
read:

void FixString(const char *strIn, char *strOut, sizet maxout);

and call it with

char *instring = "whatever";
char outstring[SZ]; /* where SZ has been suitably defined */

FixString(instring, outstring, SZ-1);

remembering that the parameters instring and outstring will
automatically be converted to pointers to the first member at the
call. You can use the maxout parameter to protect against buffer
overflow. You can also change the void type to int if you need to
return an error indicator (such as incipient buffer overflow).
--
Some informative links:
< <http://www.geocities.com/nnqweb/>
<http://www.catb.org/~esr/faqs/smart-questions.html>
<http://www.caliburn.nl/topposting.html>
<http://www.netmeister.org/news/learn2quote.html>
<http://cfaj.freeshell.org/google/>

 

[Barry Schwarz]

[snip]
My complier is Visual C++ 6 wcih I know is old but it is what was
available.

I genuinely think my problems are dwon to struggling with indirection
and pointers moret han the compiler, tempting tho it is to blame
Microsoft.

A C++ compiler should refuse to compile the line you snipped;

char *strReturn = &strTmp;

Here you are attempting to initialise a char* with a char**, either use

The advice is correct but the detail is wrong. The statement attempts
to assign a char* with a value of type char(*)[40].

char *strReturn = strTmp;

or

char *strReturn = &strTmp[0];

snip


Remove del for email

 

[Keith Thompson]

<OT>In C++, you can't assign or initialise with incompatible types, it's
an error.</OT>

I can't see why C doesn't enforce the same rules, if you want to do an
incompatible assignment, you can force the issue with a cast.

C does enforce the same rules. An assignment between incompatible
types is a constraint violation, requiring a diagnostic.

A C compiler is never required to *reject* a translation unit unless
it contains a "#error" directive. (Many implementation-specific
extensions are implemented by giving meaning to code that would
otherwise violate a constraint.)

 

[Ian Collins]

[snip]
My complier is Visual C++ 6 wcih I know is old but it is what was
available.

I genuinely think my problems are dwon to struggling with indirection
and pointers moret han the compiler, tempting tho it is to blame
Microsoft.

A C++ compiler should refuse to compile the line you snipped;

char *strReturn = &strTmp;

Here you are attempting to initialise a char* with a char**, either use

The advice is correct but the detail is wrong. The statement attempts
to assign a char* with a value of type char(*)[40].

I know, that's why I corrected it.

 

[Ian Collins]

C does enforce the same rules. An assignment between incompatible
types is a constraint violation, requiring a diagnostic.

Rather feeble enforcement.

A C compiler is never required to *reject* a translation unit unless
it contains a "#error" directive. (Many implementation-specific
extensions are implemented by giving meaning to code that would
otherwise violate a constraint.)

Or contains an expression that isn't well formed. In C++, an assignment
between incompatible types isn't well formed.

 

[Old Wolf]

Or contains an expression that isn't well formed.

No. A C compiler is not required to reject a translation unit that
contains an expression that isn't well formed. Keith Thompson's
statement was correct.

 

[Richard Heathfield]

pkirk25 said:

"0 error(s), 0 warning(s)"

Well, that means i can go to bed!

No, it means it's time to crank up your warning level.

 

[sven_tmp]

Hi pkirk25

if you write your FixString function with two pointer-Arguments,
you don't have to make a return call.

char * FixString(char *strIn, char *strOut)

Other Solution:

void FixString(char *strIn, char *strOut)
{
int i = 0;
int j = strlen(strIn);

for (i = 0; i <= j; ++i) {
/* removing the cleanup code for brevity on usenet */
strOut = strIn;
}
printf("%s\n", strOut);
}

int main(void)
{
char *strOut = "This is a line from a web page <br>";
int i = strlen(strOut);
char *backBuff = malloc(i);
FixString(strOut, backBuff);

printf("%s\n", backBack); /* Prints out the string
perfectly */

return 0;
}


Sven

 

[Richard Heathfield]

(e-mail address removed) said:

Hi pkirk25

if you write your FixString function with two pointer-Arguments,
you don't have to make a return call.


Other Solution:

void FixString(char *strIn, char *strOut)
{
int i = 0;
int j = strlen(strIn);

Undefined behaviour. Bonus question: why?

for (i = 0; i <= j; ++i) {
/* removing the cleanup code for brevity on usenet */
strOut = strIn;
}
printf("%s\n", strOut);

Undefined behaviour. Bonus question: why?

}

int main(void)
{
char *strOut = "This is a line from a web page <br>";
int i = strlen(strOut);

Undefined behaviour. Bonus question: why?

char *backBuff = malloc(i);

Constraint violation. Bonus question: why?

FixString(strOut, backBuff);

Undefined behaviour. Bonus question: why?

printf("%s\n", backBack); /* Prints out the string
perfectly */

Undefined behaviour. Bonus question: why?

return 0;
}

At least you got main's return type right.