Another problem with hex...

[Null]

Hello, I have a problem converting raw binary to hex:

#include <cstdlib>
#include <cstdio>
#include <iostream>
using namespace std;


void PrintHex()
{
     char buff[512];
     memset(buff,0,512);
    FILE *fp=fopen("C:\\new.bin","rb");
    fread(buff,512,1,fp); 
     fclose(fp);
     
     
     char hb[1024];       // hex buffer
     memset(hb,0,1024);
     char *hex=&hb[0];    // pointer to the hex buffer
     
     // reading from buff[i] and writing to hex
     for(int i=0;i<512;i++)
      {
        sprintf (hex, "%02X", buff[i]); 
        hex+=2; // oh no, pointer arithmetic!
     }
   int p=0;
   for(int i=0;i<32;i++)
     {
        for(int j=0;j<16;j++)
          {   
             cout <<" "<<hb[p]<<hb[++p];
            ++p;
          }
          cout <<endl;
     }
}     



int main()
{
    
    PrintHex();
    
   cout <<cin.get(); 
    
}   

At first it worked fine but I noticed if actual byte is 0x90 then it is displayed as 0xFF (other numbers are displayed correctly...).
So what a hell may be wrong here?
Thanks for advice!

[Disch]

I don't see what would cause $90 to screw up... but I do see a problem and one other curiosity:

The problem I see is line 31. Compound statements are evil! This line may not do what you expect

cout <<" "<<hb[p]<<hb[++p];  // <-  no wy to tell which 'hb' will be evaluated first
++p;
// might do what you expect, or might output the +1 byte twice!!
// avoid these kinds of shortcuts!  bad bad bad!

//better code:
cout << " " << hb[p] << hb[p+1];
p += 2;

The curiosity I have is your sprintf stuff. Why are you using sprintf to convert to a string, only to cout the string? why not just output the numbers directly to cout?

[Null]

I found out that everything in range of 0x7F to 0xFF is displayed as 0xFF. Strange...

The curiosity I have is your sprintf stuff. Why are you using sprintf to convert to a string, only to cout the string? why not just output the numbers directly to cout?

I'll implement this code to a GUI app later and cout is not used there.

[Disch]

I found out that everything in range of 0x7F to 0xFF is displayed as 0xFF. Strange...

AHA. I see now. I thought this might be a problem, but I didn't think it would cause the symptoms you were describing.

The problem: 'buff' is signed. Change it to 'unsigned char'.

Also, make 'hb' at least 1 char bigger (or, preferably, get rid of these functions and use stringstream instead)

-------------

Why this (and sprintf) is bad:

You are corrupting the stack. It's quite possible this code may crash your program at a future date, or cause other very weird, hard to find errors.

Effectively what you are doing is this:

char v = 0x90;
char buf[2];
sprintf(foo,"%02X",v);

Problem 1) Even if this prints as expected, 'buf' is not big enough because sprintf will print 3 characters (two digits, followed by the null terminator). IE: Buffer overflow, stack corruption.

Problem 2) '0x90' when stored in a [i]signed[i] char is -112. Not +144 like you may expect.

Problem 3) sprintf is not type safe. It has no way to know 'v' is a char. Given that you are doing "%X", it is probably assuming 'v' is an unsigned int. And -112 when cast to an unsigned int is: 0xFFFFFF90. Therefore sprintf is actually printing 9 characters: "FFFFFF90" followed by the null terminator.

IE: even more buffer overflow and stack corruption. Also, since you're only printing the first 2 characters of this sequence, that's why you keep getting FF.

[Null]

Given that you are doing "%X", it is probably assuming 'v' is an unsigned int.

This should fix the problem (I think so)

sprintf ((char*)hex, "%02Xc", buff[i]);    // changed "%02X" to "%02Xc" 

Also after changing char to unsigned char code works now.
Thanks!

[Disch]

No, that'll just put a 'c' at the end. IE: "90c" instead of "90".

Also if you didn't fix your buffer overflow problem, your code isn't fixed. You're sitting on a potential disaster. Your code might work fine now, but one day the program might just start totally breaking or acting weird and it'll be really hard to find the problem.

You really should consider switching to stringstream. sprintf is really easy to misuse and can have disasterous side effects when you misuse it.

[Null]

No, that'll just put a 'c' at the end. IE: "90c" instead of "90".

But it works for me... theres no 'c' at the end

[helios]

You're overwriting the c the next time through the loop.

[Null]

More code:

void PrintHex()
{
     char buff[512];
     memset(buff,0,512);
    FILE *fp=fopen("C:\\new.txt","rb");
    fread(buff,512,1,fp); 
     fclose(fp);
     stringstream ss;
     
     char hb[1024];       // hex buffer
     memset(hb,0,1024);
     char *hex=&hb[0];    // pointer to the hex buffer
     
     // writing to stringstream and converting to hex
     for(int i=0;i<512;i++)
      {
        ss<<hex<<buff[i]  ;   
      }
   int p=0;
   
/* I should write stringstream to hb here... but how? */


} 

int main()
{
    
    PrintHex();
    
   cout <<cin.get(); 
    
}  
 

The data in stringstream is in hex but how do I convert it to text?

And WOW! one bug fixed another bug! Cool :-]

[helios]

.str()