Returning a Char Array from function in C

Started by
10 comments, last by Vortez 10 years, 6 months ago

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


FILE *FS;
FILE *LF;


unsigned int HexDec(char Hx[16])
{
    unsigned int ret;
    ret = (long int)strtol(Hx, NULL, 16);
    return ret;
}
char DecHex(unsigned int Dec) //4 bytes
{
    char ret[16];
    sprintf(ret,"%08x", Dec);
    return ret;
}
char BinHex(char Bin[16]) //4 Bytes
{
    char ret[16];
    sprintf(ret, "%02x%02x%02x%02x", (unsigned int) Bin[0] & 0xFF, (unsigned int) Bin[1] & 0xFF, (unsigned int) Bin[2] & 0xFF, (unsigned int) Bin[3] & 0xFF);
    return ret;
}
void main()
{
    char BIN[16] = "AA55";
    char HEX[16] = BinHex(BIN);
    unsigned int DEC = HexDec(HEX);
    char DECHEX[16] = DecHex(DEC);
    printf("Bin: %s\n\n", BIN);
    printf("To Hex: %s\nTo Dec: %u\n\n Dec %u to Hex %s\n", HEX, DEC, DEC, DECHEX);
}
I am currently rewriting horrible looking test code and trying to get some functions to clean up, but I'm running into the issue where I must have arrays not pointers returned to be able to manipulate the data being processed.
?The end goal is to give me access to my custom File System threw linux terminal. Currently my first code works flawlessly, but the code is so hard to read and has the same segments over and over again (these segments are what I'm trying to get into functions).
The function DecHex is the main one I am worried about returning as an array because my code currently looks like:

//sprintf(hx,"%08x",(unsigned int) filesize);  what is currently here
hx = DecHex(filesize);
printf("Hex value of Decimal value %ld  is %s\n",filesize,hx);
sprintf (tmp, "%c%c", hx[6], hx[7]);
tmpb = strtol(tmp, NULL, 16);
printf("%s - %u\n", tmp, tmpb);
putc(tmpb, FS);
sprintf (tmp, "%c%c", hx[4], hx[5]);
tmpb = strtol(tmp, NULL, 16);
printf("%s - %u\n", tmp, tmpb);
putc(tmpb, FS);
sprintf (tmp, "%c%c", hx[2], hx[3]);
tmpb = strtol(tmp, NULL, 16);
printf("%s - %u\n", tmp, tmpb);
putc(tmpb, FS);
sprintf (tmp, "%c%c", hx[0], hx[1]);
tmpb = strtol(tmp, NULL, 16);
printf("%s - %u\n", tmp, tmpb);
putc(tmpb, FS);
And as I stated my old code is bad for the eyes (~900 lines of confusing), I am getting to that part but first I need this pointer array issue fixed.
If you need my old code for something let me know, otherwise it is best kept away from eyes lol.
Any help is appreciated, Thanks in advance B T Hoover
Advertisement
Well, for starters, you're code is currently very very wrong.

As an additional note, you cannot return arrays from functions. They will always decay to pointers. This is part of the language. So get used to using pointers. As an additional note, for C, it is a best practice to pass in pointers to the resulting storage, along with the length of said storage. Both for input and output. It eases the lifetime management and ensures that ownership of the memory is very clear. Returning a pointer from a function makes it hard to determine who owns the memory. Good examples of where this behavior has been used is the entire Win32 SDK, and the DirectX SDK, along with most of COM.

Furthermore the comments on your code are terrible. 4 bytes? What is that even supposed to mean. It takes in a parameter that has 16 bytes, it returns another that's 1 byte (but the code tries to return a pointer...), etc.

Here's your code with a bit of additional annotation.

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


FILE *FS;
FILE *LF;


unsigned int HexDec(char Hx[16])
{
    unsigned int ret;
    ret = (long int)strtol(Hx, NULL, 16); // this returns a long, not an unsigned int, you don't need that cast
    return ret;
}

char DecHex(unsigned int Dec) //4 bytes
{
    char ret[16];
    sprintf(ret,"%08x", Dec);
    return ret;  // returning a pointer to a stack allocated temporary array when you claim to be returning a char. Both are bad ideas.
}

char BinHex(char Bin[16]) //4 Bytes
{
    char ret[16];
    sprintf(ret, "%02x%02x%02x%02x", (unsigned int) Bin[0] & 0xFF, (unsigned int) Bin[1] & 0xFF, (unsigned int) Bin[2] & 0xFF, (unsigned int) Bin[3] & 0xFF);
    return ret;  // Same as above, returning stack allocated temporary, and attempting to return a pointer when you claim to be returning a char.
}

void main()
{
    char BIN[16] = "AA55";
    char HEX[16] = BinHex(BIN); // Attempting to initialize a variable with a function call that returns as pointer. This will not work.
    unsigned int DEC = HexDec(HEX);
    char DECHEX[16] = DecHex(DEC); // Attempting to initialize a variable with a function call that returns as pointer. This will not work.
    printf("Bin: %s\n\n", BIN);
    printf("To Hex: %s\nTo Dec: %u\n\n Dec %u to Hex %s\n", HEX, DEC, DEC, DECHEX);
}

In time the project grows, the ignorance of its devs it shows, with many a convoluted function, it plunges into deep compunction, the price of failure is high, Washu's mirth is nigh.

so, my hackish way got around not being able to return arrays and changed all my char functions to just a function no return and added an argument of the variable to modify which is an array.

and FYI the 4-Bytes was referring to how many bytes I hard coded the section to process, remember this is reading an entire disk with a custom File System and I'm using 32 bit addressing which breaks down to 4 bytes, actually my code can handle 1-8 bytes, but I subtracted a lot of it to save on reading time of useless information

1 sector = 512 bytes

1 cluster = 19 sectors

sector 1 = boot sector

sector 2-19 = File Table

rest = File Data

You don't return a char array from a function, you pass a pointer to that char array and it's size to a function, then do what you want with it.

Ex.

void SomeFunction(char *pArray, int size)

{

LPCSTR txt = "some text";

strncpy(pArray, "some text", size);

}

If you use sprintf, at least make sure to not overflow the buffer.

Because someone else posted in response, I'll show you my solution:


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


unsigned int HexDec(char* Hx)
{
    unsigned int ret;
    ret = (unsigned int)strtol(Hx, NULL, 16);
    return ret;
}
void DecHex(unsigned int Dec, char ret[16], int bytes)        //1-8 bytes
{
    switch(bytes)
    {
        case 1:
            sprintf(ret,"%02x", Dec);
            break;
        case 2:
            sprintf(ret,"%04x", Dec);
            break;
        case 3:
            sprintf(ret,"%06x", Dec);
            break;
        case 4:
            sprintf(ret,"%08x", Dec);
            break;
        case 5:
            sprintf(ret,"%10x", Dec);
            break;
        case 6:
            sprintf(ret,"%12x", Dec);
            break;
        case 7:
            sprintf(ret,"%14x", Dec);
            break;
        case 8:
            sprintf(ret,"%16x", Dec);
            break;
        default:
            sprintf(ret,"%08x", Dec);
            break;
    }
    return;
}
void BinHex(char Bin[16], char ret[16], int bytes)        //1-8 Bytes
{
    //int bytes = sizeof(Bin);
    switch(bytes)
    {
        case 1:
            sprintf(ret, "%02x", (unsigned int) Bin[0] & 0xFF);
            break;
        case 2:
            sprintf(ret, "%02x%02x", (unsigned int) Bin[0] & 0xFF, (unsigned int) Bin[1] & 0xFF);
            break;
        case 3:
            sprintf(ret, "%02x%02x%02x", (unsigned int) Bin[0] & 0xFF, (unsigned int) Bin[1] & 0xFF, (unsigned int) Bin[2] & 0xFF);
            break;
        case 4:
            sprintf(ret, "%02x%02x%02x%02x", (unsigned int) Bin[0] & 0xFF, (unsigned int) Bin[1] & 0xFF, (unsigned int) Bin[2] & 0xFF, (unsigned int) Bin[3] & 0xFF);
            break;
        case 5:
            sprintf(ret, "%02x%02x%02x%02x%02x", (unsigned int) Bin[0] & 0xFF, (unsigned int) Bin[1] & 0xFF, (unsigned int) Bin[2] & 0xFF, (unsigned int) Bin[3] & 0xFF, (unsigned int) Bin[4] & 0xFF);
            break;
        case 6:
            sprintf(ret, "%02x%02x%02x%02x%02x%02x", (unsigned int) Bin[0] & 0xFF, (unsigned int) Bin[1] & 0xFF, (unsigned int) Bin[2] & 0xFF, (unsigned int) Bin[3] & 0xFF, (unsigned int) Bin[4] & 0xFF, (unsigned int) Bin[5] & 0xFF);
            break;
        case 7:
            sprintf(ret, "%02x%02x%02x%02x%02x%02x%02x", (unsigned int) Bin[0] & 0xFF, (unsigned int) Bin[1] & 0xFF, (unsigned int) Bin[2] & 0xFF, (unsigned int) Bin[3] & 0xFF, (unsigned int) Bin[4] & 0xFF, (unsigned int) Bin[5] & 0xFF, (unsigned int) Bin[6] & 0xFF);
            break;
        case 8:
            sprintf(ret, "%02x%02x%02x%02x%02x%02x%02x%02x", (unsigned int) Bin[0] & 0xFF, (unsigned int) Bin[1] & 0xFF, (unsigned int) Bin[2] & 0xFF, (unsigned int) Bin[3] & 0xFF, (unsigned int) Bin[4] & 0xFF, (unsigned int) Bin[5] & 0xFF, (unsigned int) Bin[6] & 0xFF, (unsigned int) Bin[7] & 0xFF);
            break;
        default:
            sprintf(ret, "%02x%02x%02x%02x", (unsigned int) Bin[0] & 0xFF, (unsigned int) Bin[1] & 0xFF, (unsigned int) Bin[2] & 0xFF, (unsigned int) Bin[3] & 0xFF);
            break;
    }
    return;
}

works flawlessly ;) (for my situation at the very least)

?_?

?_?

LOL!

How the hell did you do that???

Anyway, i dont want to be mean, we all started like this, me included, but this should go in the coding horror section smile.png

Your code is flawed in so many way, im surprised it even work.

All this code could be baked down using loops to something like 8 lines of code...

May I suggest working on something more easier first, like strings manipulation, and integer to strings conversion and vice versa, and loops.

?_?

LOL!

How the hell did you do that???

Anyway, i dont want to be mean, we all started like this, me included, but this should go in the coding horror section smile.png

Your code is flawed in so many way, im surprised it even work.

All this code could be baked down using loops to something like 8 lines of code...

May I suggest working on something more easier first, like strings manipulation, and integer to strings conversion and vice versa, and loops.

first Alt +0CA0 in windows or &#3232; HTML code or "\u0CA0" C(++) and Java for the ? character

Yes, I know my code is way off but it compiles and links with no errors or warnings lol and works.

For me I am not used to standard libraries - or protected mode, you should see how bad the actual code looks, you just saw me trying to convert data and my code above was wrong too, BinHex was completely backwards

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)

FYI:

I am mostly doing this because I was having such a hard time trying to read the FAT12 File System and I wanted to expand my knowledge more and the best way I could think of was make my own so that I can get the in's and out's using the FAT File System as my base and changing very little.

I have one more function to get working and V 0.2 shall be done and if anyone wants it just post and I'll release it.

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;
  • 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.
*Be wary of statements such as "always do this" or "never do that", it's "always" situational and may not fit every use case.

Here's the reduced code:
[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
}
[/spoiler]

Hope this helps!

EDIT: Formatting/editing

...or better yet, use a binary format and avoid all the messy edge-cases associated with text parsing.

Direct3D has need of instancing, but we do not. We have plenty of glVertexAttrib calls.

This topic is closed to new replies.

Advertisement