Jump to content

  • Log In with Google      Sign In   
  • Create Account

We're offering banner ads on our site from just $5!

1. Details HERE. 2. GDNet+ Subscriptions HERE. 3. Ad upload HERE.


Don't forget to read Tuesday's email newsletter for your chance to win a free copy of Construct 2!


Returning a Char Array from function in C


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
11 replies to this topic

#1 BASICFreak   Members   -  Reputation: 115

Like
0Likes
Like

Posted 20 September 2013 - 08:00 PM

#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
 


Sponsor:

#2 Washu   Senior Moderators   -  Reputation: 5362

Like
3Likes
Like

Posted 20 September 2013 - 08:35 PM

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);
}


Edited by Washu, 20 September 2013 - 08:46 PM.

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.
ScapeCode - Blog | SlimDX


#3 BASICFreak   Members   -  Reputation: 115

Like
0Likes
Like

Posted 20 September 2013 - 09:20 PM

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



#4 Vortez   Crossbones+   -  Reputation: 2704

Like
0Likes
Like

Posted 21 September 2013 - 04:38 PM

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.


Edited by Vortez, 21 September 2013 - 04:39 PM.


#5 BASICFreak   Members   -  Reputation: 115

Like
0Likes
Like

Posted 21 September 2013 - 07:07 PM

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)


Edited by BASICFreak, 21 September 2013 - 07:08 PM.


#6 fastcall22   Crossbones+   -  Reputation: 4386

Like
3Likes
Like

Posted 21 September 2013 - 08:15 PM

ಠ_ಠ
c3RhdGljIGNoYXIgeW91cl9tb21bMVVMTCA8PCA2NF07CnNwcmludGYoeW91cl9tb20sICJpcyBmYXQiKTs=

#7 Vortez   Crossbones+   -  Reputation: 2704

Like
3Likes
Like

Posted 21 September 2013 - 08:24 PM

ಠ_ಠ

 

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.


Edited by Vortez, 21 September 2013 - 08:32 PM.


#8 BASICFreak   Members   -  Reputation: 115

Like
0Likes
Like

Posted 23 September 2013 - 05:03 PM

 

ಠ_ಠ

 

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.



#9 fastcall22   Crossbones+   -  Reputation: 4386

Like
0Likes
Like

Posted 23 September 2013 - 05:45 PM

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


Hope this helps!

EDIT: Formatting/editing

Edited by fastcall22, 23 September 2013 - 06:08 PM.

c3RhdGljIGNoYXIgeW91cl9tb21bMVVMTCA8PCA2NF07CnNwcmludGYoeW91cl9tb20sICJpcyBmYXQiKTs=

#10 mhagain   Crossbones+   -  Reputation: 8140

Like
1Likes
Like

Posted 24 September 2013 - 03:08 AM

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


It appears that the gentleman thought C++ was extremely difficult and he was overjoyed that the machine was absorbing it; he understood that good C++ is difficult but the best C++ is well-nigh unintelligible.


#11 BASICFreak   Members   -  Reputation: 115

Like
0Likes
Like

Posted 27 September 2013 - 12:18 PM

 

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


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



#12 Vortez   Crossbones+   -  Reputation: 2704

Like
0Likes
Like

Posted 30 September 2013 - 01:43 AM

unsigned int HexDec(char* Hx)
{
    unsigned int ret;
    ret = (unsigned int)strtol(Hx, NULL, 16);
    return ret;
}

All this can be reduced to:

UINT HexDec(char *Hx)

{

    return strtol(Hx, NULL, 16);

}

Also...

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];
...
}

This just look wrong to me and should be declared like this:

void BinHex(char *src, char *dst, int bytes)
{
...
}

// And used like this...
char Bin[17];
char ret[17];

BinHex(&Bin[0], &ret[0], some_number);


Still, it look a little better than before, keep it up.






Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS