Hand Coded State Driven Lexical Analyzer di C Dengan Unit Test Part C

Question

Ulasan ini disajikan dalam 3 pertanyaan karena jumlah kode:

1. Bagian A berisi Lexical Analyzer dan bagian utama dari kode pengujian unit.
2. Bagian B berisi pengujian unit tingkat yang lebih rendah yang disebut di Bagian A
3. Bagian C (pertanyaan ini) berisi kode pengujian unit umum yang disertakan dalam semua pengujian unit yang akan ditulis.

Perlunya Kode Tes Unit Umum

Karena saya merencanakan setidaknya 5 unit pengujian dan mungkin hingga 7 unit, ada kebutuhan nyata untuk memiliki kode umum yang dapat dibagikan oleh pengujian unit, terutama logging pengujian unit dan pelaporan kesalahan. Fungsi yang diberikan di sini digunakan secara ekstensif dalam kode pengujian unit di pertanyaan lain.

Questions

I learned C a long time ago from K&R “The C Programming Language” Version 1 (pre C89/C90).

1. Other than compiling this –O3 what can I do to optimize this code?
2. Is there archaic C usage that is not customary to use anymore?
3. Is the code readable?

Code Available:

Rather than copy and pasting this code it is available in my GitHub Repository. The code as presented in these 3 questions is on the branch Before_First_Code_Review, updates including those based on the review will be added to the master branch.

The repository structure.

common_unit_test_logic.h

#ifndef COMMON_UNIT_TEST_LOGIC_H
#define COMMON_UNIT_TEST_LOGIC_H
#include <stdio.h>
#include <stdbool.h>
#ifndef REDUCED_VM_AND_HRF_DEPENDENCIES
#include "human_readable_program_format.h"
#endif

typedef struct test_log_data
{
    char* function_name;
    bool status;
    char* path;
    bool stand_alone;
} Test_Log_Data;

extern FILE* error_out_file;
extern FILE* unit_test_log_file;

extern bool init_vm_error_reporting(char* error_log_file_name);
#ifndef REDUCED_VM_AND_HRF_DEPENDENCIES
extern Human_Readable_Program_Format* default_program(size_t* program_size);
#endif
extern void disengage_error_reporting(void);
extern bool init_unit_tests(char* log_file_name);
extern void report_error_generic(char* error_message);
extern void report_create_and_init_test_log_data_memory_failure(char* function_name);
extern void log_test_status_each_step(char* function_name, bool status, char* path, bool stand_alone);
extern void init_test_log_data(Test_Log_Data* log_data, char* function_name, bool status, char* path, bool stand_alone);
extern Test_Log_Data* create_and_init_test_log_data(char* function_name, bool status, char* path, bool stand_alone);
extern void log_test_status_each_step2(Test_Log_Data* test_data_to_log);
extern void log_start_positive_path(char* function_name);
extern void log_start_positive_path2(Test_Log_Data* log_data);
extern void log_start_test_path(Test_Log_Data* log_data);
extern void log_end_test_path(Test_Log_Data* log_data);
extern void log_end_positive_path(char* function_name);
extern void log_end_positive_path2(Test_Log_Data* log_data);
extern void log_start_negative_path(char* function_name);
extern void log_end_negative_path(char* function_name);
extern void log_generic_message(char *log_message);
extern void close_unit_tests(void);

#endif // !COMMON_UNIT_TEST_LOGIC_H

common_unit_test_logic.c

#include "common_unit_test_logic.h"
#ifndef REDUCED_VM_AND_HRF_DEPENDENCIES
#include "virtual_machine.h"
#endif
#include <ctype.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

FILE* error_out_file = NULL;
FILE* unit_test_log_file = NULL;

bool init_vm_error_reporting(char* error_log_file_name)
{
    bool status_is_good = true;

    if (error_log_file_name)
    {
        error_out_file = fopen(error_log_file_name, "w");
        if (!error_out_file)
        {
            error_out_file = stderr;
            fprintf(error_out_file, "Can't open error output file, %s", "error_log_file_name");
            status_is_good = false;
        }
    }
    else
    {
        error_out_file = stderr;
    }

    return status_is_good;
}

void disengage_error_reporting(void)
{
    if (error_out_file != stderr)
    {
        fclose(error_out_file);
    }
}

bool init_unit_tests(char* log_file_name)
{
    if (log_file_name)
    {
        unit_test_log_file = fopen(log_file_name, "w");
        if (!unit_test_log_file)
        {
            fprintf(error_out_file, "Can't open %s for output\n", log_file_name);
            return false;
        }
        error_out_file = unit_test_log_file;
    }
    else
    {
        unit_test_log_file = stdout;
        error_out_file = stderr;
    }

    return true;
}

void report_error_generic(char *error_message)
{
    fprintf(error_out_file, "%s\n", error_message);
}

void close_unit_tests(void)
{
    if (unit_test_log_file != stdout)
    {
        fclose(unit_test_log_file);
    }
}

static bool log_test_is_positive_path(Test_Log_Data* log_data)
{
    bool is_positive = true;

    if (!log_data->path)
    {
        fprintf(error_out_file, "Programmer error: log_data->path is NULL in log_test_is_positive_path()\n");
        return false;
    }

    char* string_to_test = _strdup(log_data->path);
    if (!string_to_test)
    {
        fprintf(error_out_file, "Memory Allocation error: _strdup() failed in log_test_is_positive_path()\n");
        fprintf(error_out_file, "Exiting program.\n");
        exit(EXIT_FAILURE);
    }

    char* stt_ptr = string_to_test;
    while (*stt_ptr)
    {
        *stt_ptr = (char) toupper(*stt_ptr);
        stt_ptr++;
    }

    is_positive = (strcmp(string_to_test, "POSITIVE") == 0);

    return is_positive;
}

void log_test_status_each_step(char* function_name, bool status, char* path, bool stand_alone)
{
    if (stand_alone)
    {
        fprintf(unit_test_log_file, "%s(): %s Path %s\n", function_name, path,
            (status) ? "Passed" : "Failed");
    }
}

void log_test_status_each_step2(Test_Log_Data *test_data_to_log)
{
    if (test_data_to_log->stand_alone)
    {
        fprintf(unit_test_log_file, "%s(): %s Path %s\n", test_data_to_log->function_name,
            test_data_to_log->path, (test_data_to_log->status) ? "Passed" : "Failed");
    }
}

void log_start_positive_path(char* function_name)
{
    fprintf(unit_test_log_file, "\nStarting POSITIVE PATH testing for %s\n\n",
        function_name);
}

void log_start_positive_path2(Test_Log_Data *log_data)
{
    fprintf(unit_test_log_file, "\nStarting POSITIVE PATH testing for %s\n\n",
        log_data->function_name);
}

void log_end_positive_path(char* function_name)
{
    fprintf(unit_test_log_file, "\nEnding POSITIVE PATH testing for %s\n", function_name);
}

void log_end_positive_path2(Test_Log_Data* log_data)
{
    fprintf(unit_test_log_file, "\nEnding POSITIVE PATH testing for %s, POSITIVE PATH  %s \n",
        log_data->function_name, log_data->status? "PASSED" : "FAILED");
}

void log_start_negative_path(char* function_name)
{
    fprintf(unit_test_log_file, "\nStarting NEGATIVE PATH testing for %s\n\n", function_name);
}

void log_end_negative_path(char* function_name)
{
    fprintf(unit_test_log_file, "\nEnding NEGATIVE PATH testing for %s\n", function_name);
    fflush(unit_test_log_file);        // Current unit test is done flush the output.
}

void log_start_test_path(Test_Log_Data* log_data)
{
    bool is_positive = log_test_is_positive_path(log_data);

    fprintf(unit_test_log_file, "\nStarting %s PATH testing for %s\n\n",
        is_positive ? "POSITIVE" : "NEGATIVE", log_data->function_name);
}

void log_end_test_path(Test_Log_Data *log_data)
{
    bool is_positive = log_test_is_positive_path(log_data);

    fprintf(unit_test_log_file, "\nEnding %s PATH testing for %s, Path %s\n",
        is_positive ? "POSITIVE" : "NEGATIVE", log_data->function_name,
        log_data->status ? "PASSED" : "FAILED");

    if (!is_positive)
    {
        fflush(unit_test_log_file);        // Current unit test is done flush the output.
    }
}

void log_generic_message(char* log_message)
{
    fprintf(unit_test_log_file, log_message);
}

void init_test_log_data(Test_Log_Data* log_data, char *function_name, bool status, char *path, bool stand_alone)
{
    log_data->function_name = function_name;
    log_data->status = status;
    log_data->path = path;
    log_data->stand_alone = stand_alone;
}

Test_Log_Data *create_and_init_test_log_data(char* function_name, bool status, char* path, bool stand_alone)
{
    Test_Log_Data* log_data = calloc(1, sizeof(*log_data));
    if (log_data)
    {
        init_test_log_data(log_data, function_name, status, path, stand_alone);
    }
    else
    {
        fprintf(error_out_file, "In %s calloc() failed\n", "create_and_init_test_log_data");
    }

    return log_data;
}

// provides common error report for memory allocation error.
void report_create_and_init_test_log_data_memory_failure(char *function_name)
{
    fprintf(error_out_file, "In function %s, Memory allocation failed in create_and_init_test_log_data\n", function_name);
}

Answer 1

typedef struct can reuse the name of struct

I see you often do something like:

typedef struct foo_bar {
    ...
} Foo_Bar;

It's a bit weird to use lower_case for the struct name and Upper_Case for the typedef. You can reuse the same name as the struct:

typedef struct foo_bar {
    ...
} foo_bar;

It's also common to append _t to the typedef'ed name so it is easier to identify it as a type name instead of a variable or function name, although the _t suffix is reserved by at least POSIX 1003.1.

No need to use extern for function declarations

The keyword extern is only necessary to declare variables without defining them, for function declarations there is no need, you can write for example the following in a header file:

bool init_vm_error_reporting(char* error_log_file_name);

Use const where appropriate

It seems like you avoided using const everywhere. Using it might allow the compiler to better optimize your code, and it will be able to report an error if you ever accidentily do write to a variable that shouldn't be changed. So for example:

bool init_vm_error_reporting(const char* error_log_file_name);

You can also use it for struct members:

typedef struct test_log_data
{
    const char* function_name;
    bool status;
    const char* path;
    bool stand_alone;
} test_log_data;

Optimize struct layout

The C standard mandates that the members of a struct appear in the same order in memory as they are declared. But this can result into gaps because of alignment restrictions. The above struct can be better layed out as follows:

typedef struct test_log_data
{
    const char* function_name;
    const char* path;
    bool status;
    bool stand_alone;
} test_log_data;

This saves 8 bytes on 64-bit architectures. In this particular case, it probably won't have a significant impact, but if structs get larger or you use a lot of them, you will reduce the amount of memory (bandwidth) used, and will less likely cause cache misses.

You can close stderr and stdout

It is perfectly fine to call fclose(stdout) and fclose(stderr), so the checks in disengage_error_reporting() and close_unit_tests() are not necessary.

Simplify log_test_is_positive_path()

It looks like you can replace this whole function with:

static bool log_test_is_positive_path(Test_Log_Data* log_data)
{
    return !strcasecmp(log_data, "POSITIVE");
}

Or if you can't use the POSIX strcasecmp() function, Windows provides _stricmp().

But maybe it is better to ensure the filename itself is always upper case, so you can just use strcmp()?

Avoid spending too much code on error handling unrelated to the unit tests

When there is an error internally in the unit tests, like when allocating memory for some string, don't waste lots of lines of code producing nice error messages and exitting gracefully. I particular like the BSD functions like err() for this, but to stay within the C standard, I recommend handling errors using perror() and abort() like so:

test_log_data *create_and_init_test_log_data(const char* function_name, bool status, const char* path, bool stand_alone)
{
    test_log_data* log_data = calloc(1, sizeof(*log_data));
    if (!log_data)
        perror("calloc()"), abort();

    init_test_log_data(log_data, function_name, status, path, stand_alone);
    return log_data;
}