Reputation: 4430
I'm new to C and am having trouble with strings. What I would like to do is create a string like "val1, val2, val3" in a loop.
Currently my code looks something like:
char tagstr[60] = "";
int k;
int n = 5;
for (k=0; k < n; k++) {
char temp[10] = "";
sprintf(temp, ", val%d", k);
strcat(tagstr, temp);
}
But the output of tagstr is ", val#", where # is some long integer value. I'm guessing I'm doing something wrong with pointers here but I've tried everything I can think of without success... any help would be much appreciated.
EDIT: more context, if it helps:
int tagsClosed = strlen(pch1) - strcspn(pch1, ")");
do {
if (curTag.size > 0) {
// problem section
char tagstr[60] = "";
int k;
for (k = 0; k < 5; k++) {
char temp[10] = "";
sprintf(temp, ", val%i", temp, k);
strcat(tagstr, temp);
}
// This prints out something like ", val-890132840" x 5 (same number)
printf ("String is now: %s\n", tagstr);
}
curTag = *(curTag.parent);
tagsClosed--;
} while (tagsClosed > 0);
curTag is a struct:
typedef struct Tag {
char * name;
int size; // number of children
int tagnum;
struct Tag* parent;
} Tag;
Upvotes: 4
Views: 33218
Reputation: 37214
The problem is that sprintf(temp, ", val%i", temp, k);
adds the value of temp
(which is actually the address of the first character in the array) to the string, and doesn't add the value of k
to the string at all. This should be sprintf(temp, ", val%i", k);
.
You can calculate the amount of space you'd need in advance (including zero terminator):
5+1 + 5+1 + 5+1 + 5+1 + 5+1 + 1 = 31 characters
Also; using strcat
is bad (for performance) because you'd be repeatedly searching for the end of the tagstr
and then copying the new characters to the end. It would be better to keep track of the current end of tagstr
and store then next group of characters directly at the end with no searching, no temporary string and no copying. For example:
void thing(void) {
char tagstr[60];
int pos = 0;
int k;
int n = 5;
for (k=0; k < n; k++) {
pos += sprintf(&tagstr[pos], ", val%d", k);
}
printf ("String is now: %s\n", tagstr);
}
Upvotes: 11
Reputation: 753455
If you decide you don't want the leading comma and blank, you can use a simple variation on the code you showed:
#include <stdio.h>
#include <string.h>
int main(void)
{
char tagstr[60] = "";
const char *pad = "";
int k;
int n = 5;
for (k = 0; k < n; k++)
{
char temp[10] = "";
snprintf(temp, sizeof(temp), "%sval%d", pad, k);
strcat(tagstr, temp);
pad = ", ";
}
printf("tagstr <<%s>>\n", tagstr);
return 0;
}
The output from the program was:
tagstr <<val0, val1, val2, val3, val4>>
However, your code works correctly for me, albeit with the leading comma and blank.
Upvotes: 2
Reputation: 20739
Works for me:
$ gcc -xc - && ./a.out
int main(void) {
char tagstr[60] = "";
int k;
int n = 5;
for (k=0; k < n; k++) {
char temp[10] = "";
sprintf(temp, ", val%d", k);
strcat(tagstr, temp);
}
printf("[%s]\n", tagstr);
}
[, val0, val1, val2, val3, val4]
Unless you're saying the problem is with the initial ", "
..
Upvotes: 3
Reputation: 46921
temp
isn't long enough to hold the result of your sprintf
. This is exactly why you should use snprintf
, strncat
and other variants of the string functions that take a size parameter, whenever you can.
Upvotes: 0