Parsing a fixed-size message using variable length array in C

Question

My C program has to parse a fixed-length message like this:

uint8_t message[8] = {80, 75, 73, 71, 1, 1, 1, 1};

another message could be:

uint8_t message[8] = {80, 75, 73, 71, 41, 42, 1, 1};

The message contains an ASCII string and 1 shows the end of string. So the actual length of the first message is 4 and the second message is 6.

I use this method to extract the message:

1. Counting the message length
2. Initializing a variable length array with this value.
3. Using memcpy to copy the bytes.

decoder.h

#ifndef DECODER_H_INCLUDED
#define DECODER_H_INCLUDED


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

#define LENGTH 100
void parse(uint8_t * message);
void get_address(char * addr, int size);
uint8_t get_size(void);

#endif // DECODER_H_INCLUDED

decoder.c

#include "decoder.h"


// Global variables in library
uint8_t parsed_message[LENGTH];
uint8_t len = 0;

void parse(uint8_t * message)
{
len = 0;
for (int i = 0; i < LENGTH; i++)
    {
        if (message[i] != 1)
        {
            parsed_message[i] = message[i];
            len++;
        }
        else
        {
            parsed_message[i] = '\0';
            len++;
            break;
        }

    }
}

void get_address(char * addr, int size)
{
    memcpy(addr, parsed_message, size);
}


uint8_t get_size(void)
{
    return len;
}

In the main function:

#include <stdio.h>
#include <stdlib.h>
#include "decoder.h"

int main(void) {

    uint8_t input[8] = {80, 75, 73, 71, 1, 1, 1, 1};
    parse(input);

    char addr[get_size()];
    get_address(addr, get_size());
    printf("==== %s ==== ADDRESS\n", addr);

    return 0;
}

I use GCC compiler and C99 standard in ARM-Based 32-bit micro-controller.

My question is that is this method safe? Or I should use another approach.

Please post the whole code as one, with the proper #includes also. — user228914
– user228914, Commented Nov 20, 2020 at 18:11
CodeReview is not for reviewing code snippets or hypothetical code. If you just want to know what a safe way is to copy the kinds of strings you get, try asking on StackOverflow. — G. Sliepen
– G. Sliepen, Commented Nov 21, 2020 at 11:25
To anyone in the Close Question queue, the issues with this question have been corrected. — pacmaninbw
– pacmaninbw ♦, Commented Nov 21, 2020 at 16:52

pacmaninbw · Accepted Answer · 2020-11-21 16:43:09Z

Overall Observations

The int main(void) function is correctly declared since no arguments are expected.

The code definitely follows the Single Responsibility Principle and this is good!

The use of portable include guards is good.

The program would be more flexible if it accepted the message as input from either the console or a file (would require an alternate declaration for main()).

Use System Defined Constants

The main() function could be made more readable if the system defined constants EXIT_SUCCESS and EXIT_FAILURE were used (only EXIT_SUCCESS in this case). These constants are included in the stdlib.h header file which is already included in the code.

#include <stdio.h>
#include <stdlib.h>
#include "decoder.h"

int main(void) {

    uint8_t input[8] = { 80, 75, 73, 71, 1, 1, 1, 1 };
    parse(input);

    char addr[get_size()];
    get_address(addr, get_size());
    printf("==== %s ==== ADDRESS\n", addr);

    return EXIT_SUCCESS;
}

Avoid Global Variables

It is very difficult to read, write, debug and maintain programs that use global variables. Global variables can be modified by any function within the program and therefore require each function to be examined before making changes in the code. In C global variables impact the namespace and they can cause linking errors if they are defined in multiple files. The answers in this stackoverflow question provide a fuller explanation.

In C it is possible to have a variable that is global to a file, but does not impact the entire program by using the static keyword.

static uint8_t parsed_message[LENGTH];
static uint8_t len = 0;

In the current implementation this compiles properly.

If you are really going to use global variables throughout the program they should be declared as extern variables in the header file.

#ifndef DECODER_H_INCLUDED
#define DECODER_H_INCLUDED

#include <stdint.h>

#define LENGTH 100
extern uint8_t parsed_message[LENGTH];
extern uint8_t len;

void parse(uint8_t* message);
void get_address(char* addr, int size);
uint8_t get_size(void);

#endif // DECODER_H_INCLUDED

Header Files

Only include header files that are absolutely necessary to make the code compile in header files, it is better to include the necessary header files in the C source file. This makes the code clearer in the source file. In decoder.h the only necessary header file is stdint.h. This could affect compile time and cause link errors if include guards are missing. In most implementations of C a temporary source file is generated by the C pre-processor and all header files are copied into the temporary file.

Variable Types

It isn't clear from reviewing the program why the code is using uint8_t rather than unsigned char. The overall result would be the same and less header files would be necessary.

Thanks for your answer. Data is received over network. Actually this code is part of a bigger program. My main problem is with using the VLAs. I would like to compare this method with dynamic memory allocation, i.e. using malloc. — Iman H
– Iman H, Commented Nov 21, 2020 at 18:02
And about uint8_t, the libraries are already included in my toolchain libraries. — Iman H
– Iman H, Commented Nov 21, 2020 at 18:04
@ImanH It should work with malloc() or calloc() parsed_message[] could be and allocated in parse(). The function parse could return a pointer. — pacmaninbw
– pacmaninbw ♦, Commented Nov 21, 2020 at 18:13
@ImanH C99 is the first version of C to support VLA, I really don't know how well it is supported in C. Memory allocation was the primary way to do this in C previously. — pacmaninbw
– pacmaninbw ♦, Commented Nov 21, 2020 at 20:45

Stack Exchange Network

Parsing a fixed-size message using variable length array in C

1 Answer 1

Overall Observations

Use System Defined Constants

Avoid Global Variables

Header Files

Variable Types

You must log in to answer this question.

Hot Network Questions

Parsing a fixed-size message using variable length array in C

1 Answer 1

Overall Observations

Use System Defined Constants

Avoid Global Variables

Header Files

Variable Types

You must log in to answer this question.

Related

Hot Network Questions