C Strings not returning from a function

[Thad Smith]

#define STRING_MAX_SIZE 254 /* ample headroom here */

/*
Function that
*/

You may be truncating for Usenet, but I consider the function header
to be one of the most important parts of my code. It is the contract
that I meet, a reference point to determine if a problem is incorrect
function implementation or incorrect usage. My suggestion is to take
advantage of that opportunity.

int FixString(const char *strIn, /* string to fix */
const char cToken, /* token to use */
char *strOut) /* the fixed string */
{
int i = 0;
int j = strlen(strIn);


if (j > STRING_MAX_SIZE)
{
printf("Input too big\n");
return 1;
}

While this works, my suggestion is since the maximum string length limit
is determined by the size of the array passed to strOut, the caller
should specify the length to the function, not code an arbitrary length
between the function and its caller. If a size parameter is needed, it
could be added as an additional parameter. In this case, it might be
better to have the caller verify that the input string is not longer
than the output string and stipulate that as an input requirement to use
the function.

Also, if an error is detected in a function, I prefer to pass an error
code to the caller and let the caller print an error message if
suitable. This makes the function more generic and able to work in
different environments where printf output may not be suitable. Also,
the caller has more context about the error to be able to give more
useful information. For example, in the case, strIn is simply a pointer
to a string, which could be used for many different things. The caller
should know what the string is used for, which might be a file name.
Therefore the caller might issue a message to stderr that the output
file name is too long.

 

[Fred Kleinschmidt]

pkirk25 said:


Huge win, well done.


size_t would be better. Incidentally, you already measured the length of
the
string, in main. Why not pass that length in as an argument, to save
recalculating it? (Measuring a string's length is O(n), so it's not
something you want to repeat if you can avoid it.)

Why bother with computing the length at all?
You can just as easily do something like:
char *c= strIn;
j=0;
while (*c) {
if ( j == STRING_MAX_SIZE-1 ) {
strOut[j] = '\0';
break; /* Or set j to an error code first? */
}
if ( cToken == *c) {
strOut[j] = '\0';
break;
}
strOut[j++] = *(c++);
}
return j;

If you write to strOut[j], you're in trouble.

{
/* The cleanup code */
if (cToken != strIn)
{
strOut = strIn;

Here, you write to strOut[j] (last time round the loop).

} else
{
/* the token is found */
strOut = '\0'; /* terminates the string */

Here, too.

int i = strlen(strOut);

Why int? strlen doesn't return int.

char *backBuff = malloc(STRING_MAX_SIZE);

i + 1 would have been enough. But you want to check that it worked:

if(backBuff != NULL)
{

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at above domain (but drop the www, obviously)

 

[Guest]

Harald van D?k said:

Harald van Dijk said:
Richard Heathfield wrote:

<snip>


So the whole thing has been re-worded in C99, and we have become
divided by a common language. My argument is based on C90, where it
remains valid, because malloc(i) is not a definition of malloc.

Odd and interesting. It may also be addressed by C99's 6.2.7p2; is this
in C90?
"All declarations that refer to the same object or function shall have
compatible type;
otherwise, the behavior is undefined."

It is [3.1.2.6], but in the absence of <stdlib.h> there is no prior
declaration for malloc, so it doesn't apply here.

I'm not sure what you mean. One possibility is that you're mixing up
6.2.7p2 with 6.7p4 ("All declarations in the same scope that refer to
the same object or function shall specify compatible types.").
Nope.

The
latter does not apply, but the former applies even to declarations in
different translation units.

A different translation unit is a completely different kettle of bananas.
It's compiled separately, and the compiler has no way of knowing what
declarations exist within Translation Unit A, at the time it is compiling
Translation Unit B. It may not even be the same compiler that was used for
compiling Translation Unit A. So it's hard to see how Translation Unit A's
contents can be relevant to whether a diagnostic message is required when
compiling Translation Unit B.

That's why it's not a constraint violation, just plain undefined
behaviour. No diagnostic required (but permitted as always).

 

[Simon Biber]

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

C++ compilers are off-topic here. A C compiler must emit a diagnostic
but need not refuse to compile.

char *strReturn = &strTmp;

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

It's actually a char(*)[40].

char *strReturn = strTmp;

or

char *strReturn = &strTmp[0];

Good advice.

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];

That's a syntax error. Perhaps you meant
char backBuff[someSize];
or
char *backBuff = (char[someSize]){0};
or
char *backBuff = malloc(someSize);

Unless someSize is not a macro for a constant literal integer (in which
case it should have been written in all caps), C99 is required for the
first one as it is a variable length array. C99 is definitely required
for the second one as it uses a compound literal (possibly of a VLA
type). The third form will work in C89.

 

[Richard Heathfield]

Harald van Dijk said:

[I said]

That's why it's not a constraint violation, just plain undefined
behaviour. No diagnostic required (but permitted as always).

I am delighted to announce that I have now completely lost the plot on this
thread. You have managed to spin me around and around sufficiently often
that I am quite dizzy, and I have no idea whether you are right or I am.

Fortunately for me, I don't particularly care (if you'll forgive me for
saying so - I assure you it's nothing personal), since what we are
discussing is code that we both agree is fundamentally broken, and our
dispute is only concerned with how the compiler is required/allowed to
react to it. Yes, academically it is interesting, and anyone who cares
enough will no doubt take up the argument if they disagree sufficiently
with you - but I have decided to adopt a position of "Just Don't Do That,
and the issue disappears."

 

[Guest]

Harald van Dijk said:

[I said]

That's why it's not a constraint violation, just plain undefined
behaviour. No diagnostic required (but permitted as always).

I am delighted to announce that I have now completely lost the plot on this
thread. You have managed to spin me around and around sufficiently often
that I am quite dizzy, and I have no idea whether you are right or I am.

Fortunately for me, I don't particularly care (if you'll forgive me for
saying so - I assure you it's nothing personal), since what we are
discussing is code that we both agree is fundamentally broken, and our
dispute is only concerned with how the compiler is required/allowed to
react to it. Yes, academically it is interesting, and anyone who cares
enough will no doubt take up the argument if they disagree sufficiently
with you - but I have decided to adopt a position of "Just Don't Do That,
and the issue disappears."

I have no problems with that. And regardless of whether a diagnostic is
required, I can agree that an implementation /should/ generate one.

 

[Andrew Poelstra]

*(c++)

What does this evaluate to? (Specifically, if the postfix operator is
within parentheses, does it evaluate to the old value of c or the new
one?)

 

[Thad Smith]

What does this evaluate to? (Specifically, if the postfix operator is
within parentheses, does it evaluate to the old value of c or the new
one?)

The old one. The parentheses do not change the value.s