C Concatenate string in while loop

Question

I'm trying to concatenate part of a struct with hex values. I run over every byte in the loop and convert to hex, then I want to concatenate all the hex into one long string.

However, I only end up with one value at the end of the loop. For some reason the string isnt concatenating properly. Any idea what Im doing wrong?

typedef struct OPTIONS_STR
{
    int max;
    int printName;
} OPTIONS;

void set_default_options(OPTIONS *options)
{
    options->max = -1;
    options->printName = 0;
}

void do_file(FILE *in, FILE *out, OPTIONS *options)
{
    char ch;
    int loop = 0;
    char buf[81];
    buf[0] = '\0';
    int sz1;
    int sz2;
    int sz3;

    int seeker = offsetof(struct myStruct, contents.datas);

    //find total length of file
    fseek(in, 0L, SEEK_END);
    sz1 = ftell(in);

    //find length from beggining to struct beginning and minus that from total length
    fseek(in, seeker, SEEK_SET);
    sz2 = sz1 - ftell(in);

    //set seek location at beginning of struct offset
    fseek(in, seeker, SEEK_SET);

    sz3 = sz2 + 1;
    char buffer[sz3];
    char msg[sz3];

    buffer[0] = '\0';

    while (loop < sz2)
    {
        if (loop == sz2)
        {
            break;
        }

        fread(&ch, 1, 1, in);
        sprintf(msg, "%02X", (ch & 0x00FF));
        strcpy(buffer, msg);

        ++loop;
    }
    printf("%s\n", buffer);
}

int main(int argc, const char * argv[]) {

    OPTIONS options;
    set_default_options(&options);

    const char *current = "/myfile.txt";
    FILE *f = fopen(current, "rb");
    do_file(f, stdout, &options);
    fclose(f);

};

strcpy() copies the source string to the destination string, overwriting its previous contents. You perhaps want strcat(), instead. — John Bollinger
– John Bollinger, Commented Sep 13, 2016 at 6:05
int index=0; index += sprintf(&buffer[index], "%02X", (ch & 0x00FF)); — LPs
– LPs, Commented Sep 13, 2016 at 6:05
Note, too, that you are not making your buffer large enough. Each input character requires two hex digits in your buffer, but you make only enough space for one. — John Bollinger
– John Bollinger, Commented Sep 13, 2016 at 6:06
Hi John Bollinger, thanks for confirming this, I thought this was an issue. I still wonder if I have to double the null terminator count as well. — Ke.
– Ke., Commented Sep 13, 2016 at 6:18
Hi LPs, how can I print this in hex if it is an int? int index=0; index += sprintf(&buffer[index], "%02X", (ch & 0x00FF)); — Ke.
– Ke., Commented Sep 13, 2016 at 6:22

Zan Lynx · Accepted Answer · 2016-09-13 07:13:52Z

3

Use strcat instead of strcpy. That should fix your problem.

For efficiency look into using a write pointer like char *p = buffer and advance the write position with something like p += sprintf(p, "%02X", (ch & 0x00FF))

Also your if(loop == sz2) break check is a useless duplicate of the while(loop < sz2) check. The while loop won't execute if loop is equal or bigger than sz2.

Also wondering why you use fread when you only want one character. fgetc or getc seems to be a better choice.

Also, no matter if you use fread or getc you need to check for the end of file. What if the file does not have sz2 bytes in it? Because all modern systems are multiprocess and multiuser, so someone might cut the file short after the call to ftell. You should never assume things because even if you just checked it, it can change. Making that assumption is what causes TOCTTOU (Time of Check To Time Of Use) bugs.

edited Sep 13, 2016 at 7:13

answered Sep 13, 2016 at 7:06

Zan Lynx

54.7k11 gold badges85 silver badges135 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Ke. Over a year ago

Hi Zan, understood, all of it, apart from the pointer part. How would I include that in my code? Do I declare char *p at the top, when buffer gets created?

Zan Lynx Over a year ago

@Ke. It has to come after buffer is created, so it has something to point to. And it cannot be inside the loop or it would get reset to buffer on each loop.

Ke. Over a year ago

ok I think I get the pointer thing now, and how sprintf works, so thanks for that, now I'm trying to add some extra characters to the array "\n\t\t\t\t" and it doesnt seem to be working, any idea why?

Zan Lynx Over a year ago

@Ke. It sounds to me as if you need to learn to use a debugger. You should have all the tools available to you to make this work.

Zan Lynx Over a year ago

@Ke. Also the way you ask that question is impossible for me to answer. You left out all the important details.

Manali Bhadsalkar · Accepted Answer · 2016-09-13 08:11:23Z

-1

In do_file(), you are copying the hex value for a single byte in a while loop. Thus, you should go to the next byte of character array buffer with each iteration of the while loop i.e. buffer++ or strcpy(buffer[loop], msg);

edited Sep 13, 2016 at 8:11

user1350184

answered Sep 13, 2016 at 7:09

Manali Bhadsalkar

436 bronze badges

Collectives™ on Stack Overflow

C Concatenate string in while loop

2 Answers 2

5 Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

5 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related