-3

I have been looking on internet for this and so far i just found a lot of questions for specific answer and not a general one.

i am kind of rusty on C. And i want to make a function that will return an array of char.

this is what i got and is not working. basically a way to convert a byte array to an array of chars to do atoi later..

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

char *get_char(int my_byte[], int packetsize)
{
    char *array_char=(char *) malloc(sizeof(char)*10);  //trying this but didnt work
     // char array_char[10]; //i had it like this before(was told to do it)

        for(int i=0;i<10;i++)
        {
            array_char[i]=my_byte[i]+0;
        }           

        return array_char;
    }

int main()
{

    int byte_array[]={1,2,3,4,5,6,7,8,9,0};
    char *temp;
    char data;

    temp=get_char(byte_array,10);   
    data=*temp;
    printf("String  point %s ",data);

}
jorge
  • 9
  • 3
  • You want to add the `'0'` *character*, not the number zero. After all, adding zero changes nothing. That is: `array_char[i]=my_byte[i] + '0';`. – John Bollinger Jan 27 '17 at 04:11
  • 2
    You *are* returning a (dynamically allocated) array of `char`. If you want it to contain the string, "1234567890", you should add `'0'` instead of just `0` when you set each character... and make it 11 elements long (rather than 10), with the last element set to `0` (or `'\0'`) to mark the end of the string. – Dmitri Jan 27 '17 at 04:13
  • 1
    Change like as [this](http://ideone.com/MmSvkf) – BLUEPIXY Jan 27 '17 at 04:22

3 Answers3

2

Two fixes:

  • As you want to convert to char, then

array_char[i]=my_byte[i]+0; should be array_char[i]=my_byte[i]+'0'; Note '0' is character (that will be converted to int) instead of numeric 0 (which doesn't do anything).

  • Also you must free temp pointer in main as that memory is dynamically allocated in get_char() function.

Edit: just notice another issue in your get_char()

char *array_char=(char *) malloc(sizeof(char)*10);

should be

char *array_char= malloc(sizeof(char)*(packetsize+1));

After the for loop, ensure the buffer is NUL-terminated:

array_char[packetsize] = '\0';

Notice that your packetsize is never used - you should get some compiler warning about it. It's bad to hard code 10 in your malloc - it's actually the whole idea of parsing the packetsize as a parameter - so use it properly.

artm
  • 17,291
  • 6
  • 38
  • 54
  • Also `printf("String point %s ",data);` --> `printf("String point %s ", temp);` – BLUEPIXY Jan 27 '17 at 04:23
  • 1
    would suggest in your code that `array_char[packetsize] = '\0';` instead of just 0 (just to emphasize it's NUL character) – artm Jan 27 '17 at 04:26
1

You need to watch out for these things:

  • You need to add a null-terminating character at the end of *array_char, otherwise using this pointer allocated from the heap will cause undefined behaviour.
  • You can simply allocate *array_char like this:

    char *array_char = malloc(packetsize+1);
    

    As sizeof(char) is 1, and +1 for trailing nullbyte.

  • You also don't need to cast return of malloc().

  • Instead of passing 10 as packetsize to get_char(), you should pass this size as sizeof(arr) / sizeof(arr[0], which is the calculated size of the array. This can be a size_t variable declared somewhere or even a macro.
  • malloc() needs to be checked, as it can return NULL if unsuccessful.
  • You need to free() temp at some point in the program.
  • array_char[i]=my_byte[i]+0; needs to be array_char[i]=my_byte[i]+'0'; instead, as '0' is the ascii code for a zero character.

  • char data needs to be char *data, as temp is a pointer.

    If you compile with -Wall -Wextra, you will see that this line:

    data=*temp;
    

    Is dangerous, and will trigger warnings of making pointers from integers without a cast. It will most likely lead to a segmentation fault. If temp and data are both pointers, then you can simply use:

    data=temp;
    

    Which sets data to the address of temp. Sometimes this is written as data = &(*temp);, but this is harder to read. Although their is no need for data, and using temp alone should be fine.

Your code can then look like this:

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

#define ARRAYSIZE(arr) (sizeof(arr) / sizeof(arr[0]))

char *get_char(int my_byte[], size_t packetsize) {
    char *array_char = malloc(packetsize+1);
    const char ascii = '0';
    size_t i;  

    if (!array_char) {
        printf("Cannot allocate %zu bytes\n", packetsize+1);
        exit(EXIT_FAILURE);
    }

    for(i = 0; i < packetsize; i++) {
        array_char[i] = my_byte[i] + ascii;
    }
    array_char[i] = '\0'; /* or array_char[packetsize] = '\0' */           

    return array_char;
}

int main(void) {
    int byte_array[]={1,2,3,4,5,6,7,8,9,0};
    char *temp, *data;

    temp = get_char(byte_array, ARRAYSIZE(byte_array));  
    data = temp;
    printf("String  point %s\n", data);

    printf("String converted into number = %d\n", atoi(data));

    free(temp);
    temp = NULL;

    return 0;
}

You can also look into strtol, which is better than using atoi() in terms of error checking.

Community
  • 1
  • 1
RoadRunner
  • 25,803
  • 6
  • 42
  • 75
0

It is Not Wise Idea to Return a Array From A Function. So how to return a string then? As most of libc functions use we can use some thing like that (i.e) passing a buffer along with our input and expect function to use output buffer to give us result.

Some issue to take care while coding

  1. write your logic first.
  2. try to use available functions from libc.
  3. while dealing with byte data/binary data be take precaution of buffer overflow.
  4. don't allocate in a function and de-allocate in another function.

Below is Example of your code with modification.

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

int get_char(uint8_t my_byte[], int packetsize, char *buffer, int max_buffer)
{
        int byte_itr, buf_itr;
        char temp_buf[16]={0x00};
        for(byte_itr=0, buf_itr=0; byte_itr<packetsize && max_buffer > buf_itr; byte_itr++)
        {
                memset(temp_buf, 0x00, sizeof(temp_buf));
                char temp_ch = my_byte[byte_itr];
                snprintf(temp_buf, sizeof(temp_buf), "%d", temp_ch);
                if( buf_itr+strlen(temp_buf) >=max_buffer){
                        break;
                }else{
                        buf_itr += strlen(temp_buf);
                        strcat(buffer, temp_buf);
                        if(byte_itr+1 < packetsize){
                                strcat(buffer, ",");
                                buf_itr += 1;
                        }
                }
        }

        return buf_itr;
}

int main()
{
        uint8_t byte_array[]={1,2,3,4,5,6,7,8,9,0};
        char char_array[32]={0x00};
        int len = get_char(byte_array, 10, char_array, sizeof(char_array));
        printf("String  point %s : len %d\n", char_array, len);

}

NOTE: when length return and size of output buffer same then buffer full condition happened.

rajesh6115
  • 705
  • 9
  • 21