If you can suggest an easier and\or better way of doing this please point me in the right direction. I'm only on my second build here and I know I have at least two more before I get all the features I want such as fragmentation (yea I know breaking files into parts is a bad idea [Microsoft] but I want the ability if it is needed)
Sure;*Be wary of statements such as "always do this" or "never do that", it's "always" situational and may not fit every use case.
- Always* keep the null-terminator in mind when dealing with strings: ret isn't large enough to hold 16 characters and the null terminator; sprintf is writing the null terminator in memory that doesn't belong to ret!
- Always* look for ways to reduce and refactor duplicate code: Notice the format parameter for sprintf in BinHex is twice that of bytes? You can reduce this whole case statement by adding another level of indirection; try using another sprintf to prepare the format parameter. The switch case in HexBin can also be reduced to a loop.
- Always* enforce parameter constraints. The bytes parameter can only be between 1 and 8, but you have a special behavior defined when it is not. It may be better to throw a fatal error in this case by using assert.
Here's the reduced code:
[spoiler][/spoiler]#include <assert.h> // 17 characters, to make room for the null-terminator void BinHex(char Bin[17], char ret[17], int bytes) { // Enforce constraints assert( 1 <= bytes && bytes <= 8 ); // Here, we use pointers to read and write to and from ret and Bin respectively char* dst = &ret[0]; char* src = &Bin[0]; // Equivalent to "for ( ; bytes > 0; bytes-- )" while ( bytes-- ) { // sprintf returns the number of characters written, so can use this // to our advantage and move the write pointer 2 characters for // every 2 characters written dst += sprintf(dst,"%02x",(unsigned int)(*src++) ); // "*src++" read a charater from src and moves it to the next character } } void DexHex(unsigned int Dec, char ret[17], int bytes) { assert( 1 <= bytes && bytes <= 8 ); char fmt[5]; // "%%" -> "%" // "%02i" -> zero-padded bytes*2, width of 2 // "x" -> "x" sprintf(fmt,"%%%02ix",bytes*2); // When bytes=4 and Dec=65534, "%%%02ix" -> "%08x" sprintf(ret,fmt,Dec); // "%08x" -> 0000FFFE }
Hope this helps!
EDIT: Formatting/editing
That helped a lot, thanks. I did have to edit your code to the following BinHex was slightly wrong:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>
#include <assert.h>
#include "MyFS.h"
// 17 characters, to make room for the null-terminator
void BinHex(char Bin[17], char ret[17], int bytes)
{
// Enforce constraints
assert( 1 <= bytes && bytes <= 8 );
// Here, we use pointers to read and write to and from ret and Bin respectively
char* dst = &ret[0];
char* src = &Bin[0];
// Equivalent to "for ( ; bytes > 0; bytes-- )"
while ( bytes-- )
{
// sprintf returns the number of characters written, so can use this
// to our advantage and move the write pointer 2 characters for
// every 2 characters written
dst += sprintf(dst,"%02x",(unsigned int)(src[bytes]) ); // "src[bytes]" read a charater from src and moves it to the previous character
}
}
void DecHex(unsigned int Dec, char ret[17], int bytes)
{
assert( 1 <= bytes && bytes <= 8 );
char fmt[5];
// "%%" -> "%"
// "%02i" -> zero-padded bytes*2, width of 2
// "x" -> "x"
sprintf(fmt,"%%%02ix",bytes*2); // When bytes=4 and Dec=65534, "%%%02ix" -> "%08x"
sprintf(ret,fmt,Dec); // "%08x" -> 0000FFFE
}
unsigned int HexDec(char* Hx)
{
unsigned int ret;
ret = (unsigned int)strtol(Hx, NULL, 16);
return ret;
}
But it defiantly helped with readability
Thanks,
Brian T Hoover