problem with strings

my code converts binary into hexadecimal. I pass an array that contains a string 0s and 1s to the function and it converts it to a string of characters that represent the hexadecimal value. The string is then printed. So for example for a string "01000000010000110111000000100000" it would output "40 43 70 20". My problem is that it works properly up to the last 0 and after that for some reason it prints some mess as well as an original array that was passed. Here is my code:

Code:

---

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

int hex_convert (char array[33], char final[10])
{
    int i, n, j = 0;
    int k;
    char temp[5];
   
    for (n = 0; n < 32; n = n + 4)
    {
        for (i = n; i < (n + 4); i++)
        {
            temp[j] = array[i];
            j++;
        }
        j = 0;
        if (strcmp ("0000", temp) == 0){
            final[strlen(final)] = '0';
        }else if (strcmp ("0001", temp) == 0){
            final[strlen(final)] = '1';
        }else if (strcmp ("0010", temp) == 0){
            final[strlen(final)] = '2';
        }else if (strcmp ("0011", temp) == 0){
            final[strlen(final)] = '3';
        }else if (strcmp ("0100", temp) == 0){
            final[strlen(final)] = '4';
        }else if (strcmp ("0101", temp) == 0){
            final[strlen(final)] = '5';
        }else if (strcmp ("0110", temp) == 0){
            final[strlen(final)] = '6';
        }else if (strcmp ("0111", temp) == 0){
            final[strlen(final)] = '7';
        }else if (strcmp ("1000", temp) == 0){
            final[strlen(final)] = '8';
        }else if (strcmp ("1001", temp) == 0){
            final[strlen(final)] = '9';
        }else if (strcmp ("1010", temp) == 0){
            final[strlen(final)] = 'A';
        }else if (strcmp ("1011", temp) == 0){
            final[strlen(final)] = 'B';
        }else if (strcmp ("1100", temp) == 0){
            final[strlen(final)] = 'C';
        }else if (strcmp ("1101", temp) == 0){
            final[strlen(final)] = 'D';
        }else if (strcmp ("1110", temp) == 0){
            final[strlen(final)] = 'E';
        }else if (strcmp ("1111", temp) == 0){
            final[strlen(final)] = 'F';
        }
    }
    return 0;
}

int main (void)
{
    char array[33] = "01000000010000110111000000100000";
    char final[10];
    hex_convert (array, final);
    printf ("%s\n", final);
    return 0;
}

---

and it outputs:

Code:

---

4043702??010000000100001101110000001000000

---

I double checked that I allocated enough space for the final string and everything seems fine. I can't figure out where it goes wrong

You are not marking the end of the string.

Also, using strlen() on each line like this:

Code:

---

final[strlen(final)]

---

not only means that the code is wasting time counting the length of the string (admittedly, it's not a long string, but you should be able to track what the size is). Also, if you don't ensure that you have a ending zero for your "final" string, it may not produce the right result either.

Edit: Also, temp is never terminated with a zero, so strcmp() is just luck if it matches any of your options.

--
Mats

cheers i fixed it

Good for you! Here's how I rewrote it.

Code:

---

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

void hex_convert (char * array, char * final)
{

        int i ;
        char c ;
    char * ptr = array ;
        char * outptr = final ;
        *outptr = '\0' ;
       
        if (strlen(array) % 8 != 0) {
                printf("Data is not a multiple of 8\n") ;
                return ;
        }
       
        while(*ptr) {
                c = '\0' ;
                for (i=0 ; i < 8 ; i++ ) {
                        c <<= 1 ;
                        if (ptr[i] & 1) c |= 1 ;
                }
                *outptr++ = c ;
                ptr += 8 ;
        }
        *outptr = 0 ;
}
int main (void)
{
    char array[33] = "01000000010000110111000000100000";
    char final[10];
    int i ;
        hex_convert (array, final);
    for (i = 0 ; i < strlen( final) ; i++) {
                printf ("%02X", (unsigned char) final[i]);
        }
    return 0;
}

---

better mix the char and integer the other way

Code:

---

while(*ptr) {
  c = 0;
  for (i=0 ; i < 8 ; i++ ) {
        c <<= 1 ;
        if ((ptr[i] - '0') & 1) c |= 1 ;
  }

---

Is not the result the same? I'm testing for the rightmost bit to be 1, and you are subtracting 0x30 and then testing for the rightmost bit to be 1. Same same. Why the extra step to subtract 0x30?