«

Hi all,

I am sure this is going to be another of me "close, keep trying, no
cigar" type questions -- as with all my others I have learnt tons, so
I will say thanks in advance.

I'm trying to do some string interpolation -- that is, expand certain
placeholders for other values, much like sprintf() does. Here's an
example:

static char* expand(const char *format)
{
char string[4096];
char *toReturn;

while (*format)
{
fprintf(stderr, "Looking at: %s\n", format);
int pos;

/* Until we find a % character. */
for (pos = 0; format[pos] && format[pos] != '%'; pos++);

/* Copy everything up to this into string. */
strncat(string, format, pos);

/* Advance where we're looking in format to that point. */
format += pos;

/* And try again if we have no percent character. */
if (*format != '%')
continue;

fprintf(stderr, "Before switch: %s\n", string);
format++;
switch (*format)
{
case 'n':
strcat(string, "Name");
break;
case 'c':
strcat(string, "County level address");
break;
case 'r':
strcat (string, "Restrictions");
break;
default:
break;
}

if (*format)
format++;
}
toReturn = string;
fprintf(stderr, "I am sending back: %s\n", toReturn);
return toReturn;
}

expand( "David (%n) --> Valued: [%c]" );

But as the debug via fprintf()s inside expand() shows, it seems to be
going beyonf (I assume) the NUL character -- since it reaches a point
of correctly exanding the string and keeps on going -- which isn't
right.

So any ideas much appreciated.

David

<> wrote in message
news:5bb9861b-5b2f-4a85-9fe2-...

> static char* expand(const char *format)
> {
> char string[4096];
> char *toReturn;

> toReturn = string;
> fprintf(stderr, "I am sending back: %s\n", toReturn);
> return toReturn;
> }

Check this out first. You're returning a pointer to the local array which
goes
out of existence on return.

Try changing string[] to static: static char string[4096]; so that you can
test the rest of the logic.

But this needs a better long term solution (caller provides a destination or
you need to allocate storage somewhere).

> expand( "David (%n) --> Valued: [%c]" );
>
> But as the debug via fprintf()s inside expand() shows, it seems to be
> going beyonf (I assume) the NUL character -- since it reaches a point
> of correctly exanding the string and keeps on going -- which isn't
> right.

--
Bartc

wrote:
> Hi all,
>
> I am sure this is going to be another of me "close, keep trying, no
> cigar" type questions -- as with all my others I have learnt tons, so
> I will say thanks in advance.
>
> I'm trying to do some string interpolation -- that is, expand certain
> placeholders for other values, much like sprintf() does. Here's an
> example:
>
> static char* expand(const char *format)
> {
> char string[4096];
> char *toReturn;
>
> while (*format)
> {
> fprintf(stderr, "Looking at: %s\n", format);
> int pos;
>
> /* Until we find a % character. */
> for (pos = 0; format[pos] && format[pos] != '%'; pos++);
>
> /* Copy everything up to this into string. */
> strncat(string, format, pos);

Danger, Will Robinson! The first time you get here, nothing
has ever been stored in any element of string[]. In particular,
strncat() may or may not be able to find a '\0' in it to mark the
end of "the string that's already there," and if it doesn't find
a '\0' there's no telling what might happen. (Whether you ever
get here a second time is open to question, because after the
first time you are well and truly Lost In Space.)

> /* Advance where we're looking in format to that point. */
> format += pos;
>
> /* And try again if we have no percent character. */
> if (*format != '%')
> continue;
>
> fprintf(stderr, "Before switch: %s\n", string);
> format++;
> switch (*format)
> {
> case 'n':
> strcat(string, "Name");
> break;
> case 'c':
> strcat(string, "County level address");
> break;
> case 'r':
> strcat (string, "Restrictions");
> break;
> default:
> break;
> }
>
> if (*format)
> format++;
> }
> toReturn = string;
> fprintf(stderr, "I am sending back: %s\n", toReturn);
> return toReturn;

Danger, Will Robinson! You are returning a pointer to
the first character of string[], but string[] itself will cease
to exist when the function returns. The caller will therefore
receive a pointer that it cannot use for anything.

> }
>
> expand( "David (%n) --> Valued: [%c]" );
>
> But as the debug via fprintf()s inside expand() shows, it seems to be
> going beyonf (I assume) the NUL character -- since it reaches a point
> of correctly exanding the string and keeps on going -- which isn't
> right.

Although I'm not sure just what you mean by "going beyonf
the NUL character," my suspicion is that misbehavior inside the
function is most likely due to the first error mentioned above.
Misbehavior from the second would not manifest itself until
after the function returns.

--
Eric Sosman
lid

Hi --

On Jun 6, 8:21*pm, Eric Sosman <esos...@ieee-dot-org.invalid> wrote:
>
> * * *Danger, Will Robinson! *You are returning a pointer to
> the first character of string[], but string[] itself will cease
> to exist when the function returns. *The caller will therefore
> receive a pointer that it cannot use for anything.

Grr, I feel like such an idiot. Just when I think I have finally
understood something I seem to then take six steps back and have to
start all over again. What I've done to (I hope) solve this is:

char *string = malloc(4096);

if (string == NULL)
{
// Do some error condition here.
}
memset(string, '\0', sizeof(string));

That ought to be sufficient here, right? I appreciate the this leaves
the caller with not having to allocate storage for what expand()
returns, but I prefer it this way.

> * * *Although I'm not sure just what you mean by "going beyonf
> the NUL character," my suspicion is that misbehavior inside the
> function is most likely due to the first error mentioned above.
> Misbehavior from the second would not manifest itself until
> after the function returns.

Which seems precisely correct, yes.

Thanks!

David

writes:

> Hi --
>
> On Jun 6, 8:21*pm, Eric Sosman <esos...@ieee-dot-org.invalid> wrote:
>>
>> * * *Danger, Will Robinson! *You are returning a pointer to
>> the first character of string[], but string[] itself will cease
>> to exist when the function returns. *The caller will therefore
>> receive a pointer that it cannot use for anything.
>
> Grr, I feel like such an idiot. Just when I think I have finally
> understood something I seem to then take six steps back and have to
> start all over again. What I've done to (I hope) solve this is:
>
> char *string = malloc(4096);
>
> if (string == NULL)
> {
> // Do some error condition here.
> }
> memset(string, '\0', sizeof(string));

That might work, but for the wrong reason. `string' is a pointer, not
an array, so sizeof(string) is the size of a pointer (probably 4 or 8
bytes), and not 4096.

However, you should really only need to set the first character of
`string' to '\0'. Then strcat and friends will correctly treat it as an
empty string.

A better approach might be to build the string in a local array, and
only when finished copy it to a malloc'ed array of the correct size.
That way you don't waste space unnecessarily.

char *expand(char *format) {
char str[4096] = ""; /* initialize to empty string */
strncat(str, "..."); /* etc */
char *p = malloc(strlen(str) + 1);
if (!p) return NULL;
strcpy(p, str);
return p;
}

On Jun 6, 3:33*pm, david.chant...@googlemail.com wrote:
> Hi --
>
> Grr, I feel like such an idiot. *Just when I think I have finally
> understood something I seem to then take six steps back and have to
> start all over again. *

Don't beat yourself up. The joy and the sorrow of C is that it's a
fairly thin abstraction over the hardware. This requires you to keep
track of hardware-ish details that a more abstract language would
never let you see or express at all. When you're learning, it can
seem like whack-a-mole.

What I've done to (I hope) solve this is:
>
> char *string = malloc(4096);
>
> if (string == NULL)
> {
> * * * // Do some error condition here.}
>
> memset(string, '\0', sizeof(string));

Others have pointed out the error here. I won't do it again. I'll
point out, however, that returning a malloc'ed block is overkill
here. You can get what you want by requiring the user to allocated
the output buffer and pass it in. For convenience you can return a
pointer to the user's buffer, so the function can be called as an
actual parameter to another function. I.e. say:

static char* expand(char *buf, const char *format)
{

... yada yada

return buf;
}


Now the caller can say:

char buf[4096];

printf("result is %s\n", expand(buf, "Go for %n."));

Another point is that after practice, you'll develop character
manipulation idioms that are easy for your head to wrap around.
Personally the technique you are using of "scan and block copy" has
always seemed awkward to me. My head finds it easier to scan and copy
at the same time. I'd set it up this way:

char* expand(char *buf, const char *fmt)
{
int ibuf = 0;
int ifmt = 0;
char ch;

#define GET(C) do { C = fmt[ifmt++]; } while (0)
#define PUT(C) do { buf[ibuf++] = (C); } while (0)
#define PUTS(S) do { \
strcpy(&buf[ibuf], (S)); ibuf += strlen(S); \
} while (0)

for (; {
GET(ch);
if (ch == '%') {
GET(ch); // Escape character.
switch (ch) {
case 'n':
PUTS("Name");
break;
case 'c':
PUTS("Country");
break;
case 'r':
PUTS("Restrictions");
break;
case '\0':
PUT('\0'); // Probably an error case.
return buf;
default:
fprintf(stderr,
"warn: bad escape %%%c @ fmt[%d]\n",
ch, ifmt);
break;
}
}
else {
// Any other character: copy and quit if it was null.
PUT(ch);
if (ch == '\0')
return buf;
}
}
}