Этот это реализация Ассемблера, необходимая для синтаксического анализа исходного кода, написанного на Взломать машинный язык и вывести его в 16-битный двоичный файл.

Написав один раз на Swift, я решил, что хочу попробовать написать его на C, чтобы стать более комфортным с языком, а также воспользоваться возможностью написать свою собственную хеш-таблицу.

Мне бы хотелось получить отзывы по любому аспекту кода, но я особенно ищу области улучшения в отношении указателей и управления памятью. Я не верю, что Valgrind поддерживается на машинах Mac M1, поэтому распределение и освобождение памяти было написано несколько слепым.

Я также был бы признателен за любые указатели (без каламбура) о том, как я могу улучшить соблюдение соглашений C.

Assemble.c

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

#include "error/Error.h"
#include "components/Stripper.h"
#include "components/Parser.h"

#define MAX_ASSEMBLED_SIZE 1048576

static void assemble(const char *file, char *output_name);

// Entry point for program
int main(int argc, char** argv) {
    
    switch (argc) {
        case 2:
            // No file name specified - revert to "assembled.hack"
            assemble(argv[1], NULL);
            break;
        case 3:
            // Use client-specified file name
            assemble(argv[1], argv[2]);
            break;
        default:
            // ❌
            exit_with_error(INCORRECT_ARGUMENT_COUNT);      
    }
    return 0;

}

// Assembles a file into binary code
static void assemble(const char *file_name, char *output_name) {

    long size_in_bytes;
    char* file_to_assemble;
    printf("Starting assembly of file %sn", file_name);
    
    // Check if the file exists
    FILE *file = fopen(file_name, "r");
    if (file == NULL) {
        exit_with_error(FILE_NOT_FOUND);
    }
    else {

        // Create a HashMap to store variables and label definitions
        HashMap* hash_map = hash_map_create();

        // Retrieve the size of the file (max 500k)
        fseek(file, 0, SEEK_END);
        size_in_bytes = ftell(file);

        if (size_in_bytes > MAX_ASSEMBLED_SIZE / 2) {
            exit_with_error(FILE_TOO_LARGE);
        }
        
        fseek(file, 0, SEEK_SET); 
        file_to_assemble = malloc(size_in_bytes + 1);

        if (file_to_assemble) {
            
            // Read the contents of the file into the buffer
            fread(file_to_assemble, 1, size_in_bytes, file);
            char no_comments[size_in_bytes + 1];
            char no_spaces[size_in_bytes + 1];
            char no_labels[size_in_bytes + 1];
            char parsed[MAX_ASSEMBLED_SIZE + 1];
            
            // Remove comments and blank spaces. Save and remove labels, save variables
            strip_comments(no_comments, file_to_assemble);
            strip_spaces(no_spaces, no_comments);
            strip_labels(no_labels, no_spaces, hash_map);
            save_variables(no_labels, hash_map);

            // Translate the remaining assembly code to binary
            parse(parsed, no_labels, hash_map);

            char output_file_name[256];

            // If the client chose a custom output name
            if (output_name != NULL) {
                strcpy(output_file_name, output_name);
            }
            else {
                strcpy(output_file_name, "assembled.hack");
            }
            
            // Write the file
            FILE *output = fopen(output_file_name, "w");
            fwrite(parsed, 1, strlen(parsed), output);
            fclose(file);
            hash_map_free(hash_map);
            printf("Assembly complete");
        }
        else {
            // Could not open file. Bail
            exit_with_error(FILE_READ_ERROR);
        }
    }
}

Stripper.c

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <stdbool.h>
#include <ctype.h>
#include "HashMap.h"
#include "Parser.h"

#define SCREEN_ADDRESS 16384
#define KBD_ADDRESS 24576
#define SP_ADDRESS 0
#define LCL_ADDRESS 1
#define ARG_ADDRESS 2
#define THIS_ADDRESS 3
#define THAT_ADDRESS 4

// Removes extraneous white space and blank lines
void strip_spaces (char* dst, const char* src) {

    bool have_reached_printable_char = false;
    int count = 0;
    int length = strlen(src);

    while(*src != '') {
        if (count == length - 1 && *src == 'n')
            break;
        have_reached_printable_char = have_reached_printable_char ? true : isprint(*src);
        if(have_reached_printable_char && (*src == 'n' || !isspace(*src))) {
            *dst = *src; // then copy
            dst++;
        }
        count++;
        src++;
    }

    *dst="";
}

// Remove comments (like this!) from file
void strip_comments(char* dst, const char* src) {

    bool copy = true;

    for (int i = 0; i < strlen(src); i++) {
        if (src[i] == 'n')
            copy = true;
        if (src[i] == "https://codereview.stackexchange.com/" && src[i+1] == "https://codereview.stackexchange.com/")
            copy = false;
        if (copy) {
            *dst = src[i];
            dst++;
        }
    }
    *dst="";
}

// Map particular variables to corresponding address
static void map_reserved_variables(HashMap* hash_map) {
    
    hash_map_put(hash_map, "screen", SCREEN_ADDRESS);
    hash_map_put(hash_map, "kbd", KBD_ADDRESS);
    hash_map_put(hash_map, "sp", SP_ADDRESS);
    hash_map_put(hash_map, "lcl", LCL_ADDRESS);
    hash_map_put(hash_map, "arg", ARG_ADDRESS);
    hash_map_put(hash_map, "this", THIS_ADDRESS);
    hash_map_put(hash_map, "that", THAT_ADDRESS);

    for (int i = 0; i < 16; i++) {
        int length = i < 10 ? 2 : 3;
        char reg[length + 1];
        reg[0] = 'r';
        sprintf(reg + 1, "%d", i);
        hash_map_put(hash_map, reg, i);
    }
}

    

// Remove label definitions and replace them with corresponding line number of
// logic following the definition
void strip_labels(char* dst, const char* src, HashMap* hash_map) {

    map_reserved_variables(hash_map);

    int current_command = 0;
    bool save_command = false;
    bool new_command = true;
    bool copy = true;
    char current_label[256];
    int current_label_index = 0;
    char last_copied;

    while(*src != '') {
        if (*src == 'n') {
            new_command = true;
            if (last_copied == 'n') {
                src++;
            }
            current_command++;
            copy = true;
        }

        if (*src == ')' && save_command) {  
            save_command = false;
            current_label[current_label_index] = '';
            // Move backwards to go back to the command we were dealing with
            current_command--;
            for (int i = 0; i <= strlen(current_label); i++) {
                char lowered = tolower(current_label[i]);
                current_label[i] = lowered;
            }
            // Now move forward one line and save whatever command number that is
            hash_map_put(hash_map, current_label, current_command+1);
            current_label_index = 0;
        }
        if (save_command) {
            current_label[current_label_index++] = *src;
        }
        if (new_command && *src == '(') {
            save_command = true;
            copy = false;
        }       
        if (copy) {
            *dst = *src;
            dst++;
            last_copied = *src;
        }
        src++;
    }
        *dst="";
}

// Save any user declared variables
void save_variables(char* dst, HashMap* hash_map) {
    bool is_a_variable_declaration = false;
    char current_variable[256];
    int current_variable_index = 0;
    int current_variable_address = 16;

    while(*dst != '') {

        if (*dst == 'n') {
            if (is_a_variable_declaration) {
                is_a_variable_declaration = false;
                current_variable[current_variable_index] = '';
                current_variable_index = 0;

                if (!is_integral_string(current_variable)) {
                    if (hash_map_contains(hash_map, current_variable)) {
                        // It's a label declaration that we've already saved in the method above
                        continue;
                    }
                    else {
                        hash_map_put(hash_map, current_variable, current_variable_address++);
                    }
                }
            }
        }
        if (is_a_variable_declaration) {
            current_variable[current_variable_index++] = tolower(*dst);
        }
        if (*dst == '@') {
            char next = tolower(*(++dst));
            if (next != 'r' && next != '0') {
                is_a_variable_declaration = true;
            }
            dst--;
        }
        dst++;
    }
}

Parser.c

#include <stdbool.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <ctype.h>
#include "HashMap.h"
#include "Parser.h"
#include "../error/Error.h"

#define VALID_DESTINATION_COUNT 7
#define VALID_COMPUTATION_COUNT 28
#define VALID_JUMP_COUNT 7
#define WORD_LENGTH 16

#define A_START 0

#define C_A_OR_M_START 3
#define C_A_OR_M_BIT_LENGTH 1

#define C_DEFAULTS_START 0
#define C_DEFAULTS_BIT_LENGTH 3
#define C_DEFAULTS_ADDRESS 7

#define C_COMP_START 4
#define C_COMP_BIT_LENGTH 6

#define C_JUMP_START 13
#define C_JUMP_BIT_LENGTH 3

#define C_DEST_BIT_LENGTH 3
#define C_DEST_START 10

// Declarations

static bool is_a_command(const char* str); 

static int jump_address(const char* str);
static int destination_address(const char* str);
static int get_computation_address(const char* cmd);
static int get_jump_address(const char* cmd, int jump_operation_position); 
static int get_destination_address(const char* cmd, int assignment_operation_position);

static void parse_c_command(char* dst, const char* cmd);
static void parse_command(char* dst, const char *cmd, HashMap* hash_map); 
static void parse_a_command(char* dst, const char* cmd, HashMap* hash_map); 
static void to_bin(char* cmd, int address, int number_of_places, int starting_position); 
static void get_positions(const char* cmd, int* assignment, int* computation, int *termination, int* jump); 


// *********** CONSTANTS ************** //

// Allowed destinations in C instruction
const char* valid_destinations[VALID_DESTINATION_COUNT] = { "M", "D", "MD", "A", "AM", "AD", "AMD" };
// Allowed computations in C instruction
const char* valid_computations[VALID_COMPUTATION_COUNT] = { "0", "1", "-1", "D", "A", "!D", "!A", "-D", 
                                                            "-A", "D+1", "A+1", "D-1", "A-1", "D+A", "D-A", 
                                                            "A-D", "D&A", "D|A", "M", "!M", "-M", "M+1", 
                                                            "M-1", "D+M", "D-M", "M-D", "D&M", "D|M" };
// Denary representation of a valid computation. 
// For example the instruction D|M (see above) corresponds to 21 -> (101001)
const int valid_computation_integers[VALID_COMPUTATION_COUNT] = { 42, 63, 58, 12, 48, 13, 49, 15, 
                                                                    51, 31, 55, 14, 50, 2, 19,
                                                                    7, 0, 21, 48, 49, 51, 
                                                                    55, 50, 2, 19, 7, 0, 21 }; 
// Allowed jump commands in C instruction
const char* valid_jumps[VALID_JUMP_COUNT] = { "JGT", "JEQ", "JGE", "JLT", "JNE", "JLE", "JMP" };


// *********** PARSING ************** //

// Parse a source asm file that has been stripped of whitespace and comments
void parse(char* dst, char* src, HashMap* hash_map) {

    char current_label[256];
    int current_label_position = 0;
    int dest_position = 0;

    for (int i = 0; i <= strlen(src); i++) {
    
        if (src[i] == 'n' || src[i] == '') {
            // We've either reached a line break or EOF
            current_label[current_label_position] = '';
            char parsed[WORD_LENGTH + 1];
            to_bin(parsed, 0, WORD_LENGTH, 0);
            parsed[WORD_LENGTH] = '';
            // Parse the current command
            parse_command(parsed, current_label, hash_map);
            for (int j = 0; j < strlen(parsed); j++) {
                // Add the newly parsed command to the output file
                dst[dest_position++] = parsed[j];   
            }
            // Reset label posiion and add n or  to output
            current_label_position = 0;
            dst[dest_position++] = src[i];
            continue;
        }
        current_label[current_label_position++] = src[i];
    }
    // Done, make sure to end with null terminator
    dst[dest_position] = '';
}

static void parse_command(char* dst, const char *cmd, HashMap* hash_map) {

    // Can either be A command ("@command") or C command ("D=D+1;JGE")
    if (is_a_command(cmd)) {
        parse_a_command(dst, cmd, hash_map);
    }
    else {
        parse_c_command(dst, cmd);
    }
}

static void parse_c_command(char* dst, const char* cmd) {

    int assignment_operation_position = 0;
    int computation_operation_position = 0;
    int jump_operation_position = 0;
    int computation_termination_position = 0;

    // Fill in parts of parsed command that are common to all C instructions
    to_bin(dst, C_DEFAULTS_ADDRESS, C_DEFAULTS_BIT_LENGTH, C_DEFAULTS_START);
    get_positions(cmd, &assignment_operation_position, &computation_operation_position, &computation_termination_position, &jump_operation_position);

    // Split the command into destination (if applicable), computation and jump (if applicable)

    if (assignment_operation_position != 0) {
        int address = get_destination_address(cmd, assignment_operation_position);
        if (address == -1) {
            exit_with_error(MALFORMED_DESTINATION);
        }
        else { 
            to_bin(dst, address, C_DEST_BIT_LENGTH, C_DEST_START);
        }
    }

    int computation_string_length = computation_termination_position - computation_operation_position;
    char computation_string[computation_string_length + 1];
    strncpy(computation_string, cmd+computation_operation_position, computation_string_length);
    computation_string[computation_string_length] = '';
    int computation_address = get_computation_address(computation_string);

    if (computation_address == -1) {
        exit_with_error(MALFORMED_COMPUTATION);
    }
    else {
        to_bin(dst, computation_address, C_COMP_BIT_LENGTH, C_COMP_START);
        for (int i = computation_operation_position; i < computation_termination_position; i++) {
            if (tolower(cmd[i]) == 'm') {
                to_bin(dst, 1, C_A_OR_M_BIT_LENGTH, C_A_OR_M_START);
            }
        }
    }
    
    if (jump_operation_position != 0) {
        int address = get_jump_address(cmd, jump_operation_position);
        if (address == -1) {
            exit_with_error(MALFORMED_JUMP);
        }
        else {
            to_bin(dst, address, C_JUMP_BIT_LENGTH, C_JUMP_START);
        }
    }
}

static void parse_a_command(char* dst, const char* cmd, HashMap* hash_map) {

    size_t cmd_length = strlen(cmd);
    char lowered[cmd_length];
    int index = 0;

    // Disregard '@' by starting at index 1
    for (size_t i = 1; i <= cmd_length; i++) {
        lowered[index++] = tolower(cmd[i]);
    }
    if (!is_integral_string(lowered)) {
        // It's a user-declared variable
        int value = hash_map_get(hash_map, lowered);
        to_bin(dst, value, WORD_LENGTH, A_START);
    }
    else {
        // It's a direct address (eg '@42')
        char address_string[cmd_length];
        strncpy(address_string, lowered, cmd_length);
        int address = atoi(address_string);
        to_bin(dst, address, WORD_LENGTH, A_START);
    }   
}


// *********** ADDRESSES ************** //

// Retreive generic address from array
static int get_address(const char* str, const char** argv, unsigned int count) {

    for (int i = 0; i < count; i++) {
        if (strcmp(str, argv[i]) == 0) {
            return i + 1;
        }
    }
    return -1;
}

// Retrieve destination address from C instruction
static int get_destination_address(const char* cmd, int assignment_operation_position) {

    char destination[assignment_operation_position + 1];
    strncpy(destination, cmd, assignment_operation_position);
    destination[assignment_operation_position] = '';
    return destination_address(destination);
}

static int destination_address(const char* str) {
    return get_address(str, valid_destinations, VALID_DESTINATION_COUNT);
}

// Retrieve computation address from C instruction
static int get_computation_address(const char* cmd) {

    for (int i = 0; i < VALID_COMPUTATION_COUNT; i++) {
        if (strcmp(valid_computations[i], cmd) == 0) {
            return valid_computation_integers[i];
        }
    }
    return -1;
}

// Retrieve jump address from C instruction
static int get_jump_address(const char* cmd, int jump_operation_position) {

    int jump_operation_length = strlen(cmd) - jump_operation_position;
    char jump_operation[jump_operation_length + 1];
    strncpy(jump_operation, cmd+jump_operation_position, jump_operation_length);
    jump_operation[jump_operation_length] = '';
    return jump_address(jump_operation);

} 

static int jump_address(const char* str) {
   return get_address(str, valid_jumps, VALID_JUMP_COUNT);
}


// *********** HELPER ************** //

static bool is_a_command(const char* str) {
    return str[0] == '@' && strlen(str) > 1;
}

// Is the command pure integers?
bool is_integral_string(char *str) {

    size_t length = strlen(str);
    for (int i = 0; i < length; i++) {
        if (!isdigit(str[i])) {
            return false;
        }
    }
    return true;
}

// Retrieve positions of assignment, computation, 
// and jump instructions as well as the point of termination of a computation
static void get_positions(const char* cmd, int* assignment, int* computation, int *termination, int* jump) {

    size_t length = strlen(cmd);
    for (int i = 0; i < length; i++) {
        if (cmd[i] == '=') {
            *assignment = i;
            *computation = i+1;
        }
        if (cmd[i] == ';') {
            *jump = i+1;
            *termination = i;
        }
    }
    if (*termination == 0) {
        *termination = length;
    }
}

// Convert an address to binary padding with number of places, 
// starting (0 being the 'end' of the string) at starting_position
static void to_bin(char* cmd, int address, int number_of_places, int starting_position) {

    int quo = address;
    int index = number_of_places + starting_position - 1;

    int rem;
    for (int i = 0; i < number_of_places; i++) {
        rem = quo % 2;
        cmd[index--] = rem == 1 ? '1' : '0';
        quo = quo / 2;
    }
}

HashMap.c

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

// Hashing function, courtesy of
// http://www.cse.yorku.ca/~oz/hash.html
static int hashed(char* arg) {

    unsigned long hash = 5381;
    int c;
    while ((c = *arg++))
        hash = ((hash << 5) + hash) + c; 
    return hash % NUM_BUCKETS;
}

// Create a HashMap (caller to free) 
HashMap* hash_map_create() {
    HashMap *hash_map = malloc(sizeof(HashMap));
    for (int i = 0; i < NUM_BUCKETS; i++)
        hash_map->buckets[i] = NULL;
    return hash_map;
}

// Insert into HashMap (assumes non-negative value)
void hash_map_put(HashMap* hash_map, char* key, int value) {

    int hashed_key = hashed(key);
    int result = hash_map_get(hash_map, key);

    if (result != -1) {
        // Overwrite old value
        Node *current = hash_map->buckets[hashed_key];
        while (current->key != key) {
            current = current->next;
        }
        current->value = value;
    }
    else {
        if (hash_map->buckets[hashed_key] == NULL) {
            // Insert at first node in bucket
            hash_map->buckets[hashed_key] = malloc(sizeof(Node));
            hash_map->buckets[hashed_key]->key =  malloc(sizeof(char) * (strlen(key) + 1));
            strcpy(hash_map->buckets[hashed_key]->key, key);
            hash_map->buckets[hashed_key]->value = value;
            hash_map->buckets[hashed_key]->next = NULL;
        }
        else {
            // Collision, traverse to end of linked list
            Node *current = hash_map->buckets[hashed_key];
            Node *new = malloc(sizeof(Node));
            new->key = malloc(sizeof(char) * (strlen(key) + 1));
            strcpy(new->key, key);
            new->value = value;
            new->next = NULL;
            while (current->next != NULL) {
                current = current->next;
            }
            current->next = new;
        }
    }
}

bool hash_map_contains(HashMap* hash_map, char* key) {
    return hash_map_get(hash_map, key) != -1;
}

// Retrieve value for key from HashMap
int hash_map_get(HashMap* hash_map, char* key) {

    int hashed_key = hashed(key);
    Node *current = hash_map->buckets[hashed_key];
    int returned = -1;

    while (current != NULL) {
        if (strcmp(current->key, key) == 0) {
            returned = current->value;
            break;
        }
        current = current->next;
    }
    return returned;
}

// Remove value for key in HashMap
void hash_map_remove(HashMap* hash_map, char* key) {

    int hashed_key = hashed(key);
    if (hash_map_get(hash_map, key) != -1) {
        Node *current = hash_map->buckets[hashed_key];
        if (current->key == key) {
            // Remove from head node
            Node *new_current = current->next;
            current = NULL;
            hash_map->buckets[hashed_key] = new_current;
        }
        else {
            // Traverse and remove when found
            while (current->next->key != key) {
                current = current->next;
            }
            Node *old_next = current->next;
            Node *new_next = current->next->next;
            current->next = new_next;
            old_next = NULL;
        }
    }
}

// Free HashMap
void hash_map_free(HashMap* hash_map) {
    free(hash_map);
}

Error.c

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

void error_string_for_code(int code, char **star);

void exit_with_error(int code) {
    char *str = "";
    error_string_for_code(code, &str);
    printf("Exiting with error: '%s'n", str);
    exit(code);
}

void error_string_for_code(int code, char **str) {
    
    switch (code) {
        case INCORRECT_ARGUMENT_COUNT:
            *str = "Incorrect argument count";
            break;
        case FILE_NOT_FOUND:
            *str = "The file doesn't exist";
            break;
        case FILE_READ_ERROR:
            *str = "Could not read file";
            break;
        case NULL_POINTER:
            *str = "Null pointer found";
            break; 
        case UNKNOWN_COMMAND:
            *str = "Unrecognized command";
            break;
        case MALFORMED_DESTINATION:
            *str = "Invalid destination received";
            break;
        case MALFORMED_COMPUTATION:
            *str = "Received a missing computation";
            break;
        case MALFORMED_JUMP:
            *str = "Invalid jump received";
            break;
        case FILE_TOO_LARGE:
            *str = "File too large, please ensure it is less than 500kb";
            break;
        default:
            *str = "Unknown error.";
        break;
    }
}
```

Вот некоторые вещи, которые могут помочь вам улучшить вашу программу.

Исправить ошибку

В assemble() файл читается с fread что уместно, но позже, когда буфер обрабатывается для удаления комментариев, код делает неверное предположение, что буфер завершается с помощью NUL персонаж. Чтобы исправить это, явно завершите строку:

// Read the contents of the file into the buffer
fread(file_to_assemble, 1, size_in_bytes, file);
// assure the buffer is terminated
file_to_assemble[size_in_bytes] = '';

Не допускайте утечки памяти

В hash_map_free() функция освобождает общий HashMap но не может освободить память каждого из Nodeс key а также *next ценности. Точно так же внутри hash_map_remove, указатель удаленного узла устанавливается на NULL что нормально, но память не освобождается, что не нормально. Вот один из способов переписать hash_map_free():

void hash_map_free(HashMap* hash_map) {
    if (hash_map) {
        for (int i = 0; i < NUM_BUCKETS; ++i) {
            for (Node* curr = hash_map->buckets[i]; curr; ) {
                Node* victim = curr;
                curr = curr->next;
                free(victim->key);
                free(victim);
            }
        }
    }
    free(hash_map);
}

Аналогичным образом вам нужно будет внести соответствующие изменения в hash_map_remove.

Понять идиому языка Си

В strip_comments функция работает, но использует индексированную переменную s[i] вместо простого указателя. Вот альтернативная версия, которая пропускает вызов strlen и в основном избегает использования индексации.

void strip_comments(char* dst, const char* src) {
    for (bool copy = true; *src; ++src) {
        if (*src == 'n') {
            copy = true;
        }
        else if (*src == "https://codereview.stackexchange.com/" && src[1] == "https://codereview.stackexchange.com/") {
            copy = false;
        }
        if (copy) {
            *dst++ = *src;
        }
    }
    *dst="";
}

Другие улучшения использовать else поскольку оба утверждения являются взаимоисключающими, и добавить {} к каждому пункту в целях безопасности. Кроме того, в этом коде используется еще одна очень распространенная идиома: *dst++ = *src;.

Используйте согласованное форматирование

в Stripper.c файла форматирование трех функций немного отличается. Читателю помогает последовательное форматирование.

Упростите выражения

В strip_spaces функция в настоящее время имеет эту строку:

have_reached_printable_char = have_reached_printable_char ? true : isprint(*src);

Мне это кажется немного странным, и мне потребовалось время, чтобы посмотреть, что он делает. Я думаю, что более четкий способ сделать это будет следующим:

have_reached_printable_char |= isprint(*src);

Использовать const где это возможно

Многие функции правильно используют const как часть их подписей, что хорошо. Однако можно сделать больше, например объявить length в strip_spaces() в качестве const.

Предпочитать for к while где это возможно

В strip_spaces функция имеет while цикл, который, я думаю, лучше было бы записать как for петля. В for цикл имеет преимущества, заключающиеся в том, что объем count сводится только к циклу (сокращение объема — это хорошо, потому что оно помогает компилятору распределять регистры и упрощает понимание людьми) и дает понять, какие значения безоговорочно обновляются в каждом цикле. В этом случае я бы написал это так:

for (int count = 0; *src != ''; ++count, ++src) {

Упростите свои функции

В for петля внутри map_reserved_variables немного сложнее, чем должно быть. Во-первых, нам все равно, насколько велик буфер, если он имеет емкость для хранения имени регистра максимального размера. Во-вторых, имя всегда начинается с r. Мы можем упростить, написав цикл следующим образом:

char reg_name[5];
for (int i = 0; i < 16; ++i) {
    snprintf(reg_name, 5, "r%d", i);
    hash_map_put(hash_map, reg_name, i);
}

Также обратите внимание на использование snprintf для дополнительной безопасности.

Сократите использование «магических чисел»

В коде есть несколько цифр, например 16 а также 256 которые имеют особое значение в их конкретном контексте. Используя именованные константы, такие как NUM_REGISTERS или MAX_LABEL_SIZE, программу становится проще читать и поддерживать. Кроме того, обычно лучше использовать const int чем #define для числовых значений.

Использовать struct связать связанные элементы

Во время синтаксического анализа get_computation_address() функция вызывается. Сейчас это так:

static int get_computation_address(const char* cmd) {
    for (int i = 0; i < VALID_COMPUTATION_COUNT; i++) {
        if (strcmp(valid_computations[i], cmd) == 0) {
            return valid_computation_integers[i];
        }
    }
    return -1;
}

Он основан на трех разных вещах: VALID_COMPUTATION_COUNT что является константой, valid_computations который представляет собой массив командных строк, и valid_computation_integers который представляет собой массив чисел, связанных с каждой командой. Вместо трех отдельных элементов со слабой привязкой между ними я бы предложил использовать struct:

const struct computation {
    const char *text;
    int value;
} Computations[] = {
    { "0", 42 }, { "1", 63 }, { "-1", 58 }, { "D", 12 }, { "A", 48 }, 
    { "!D", 13 }, { "!A", 49 }, { "-D", 15 }, { "-A", 51 }, {"D+1", 31 }, 
    {"A+1", 55 }, {"D-1", 14 }, {"A-1", 50 }, {"D+A", 2 }, {"D-A", 19 },
    { "A-D", 7 }, {"D&A", 0 }, {"D|A", 21 }, {"M", 48 }, {"!M", 49 }, 
    {"-M", 51 }, {"M+1", 55 }, { "M-1", 50 }, {"D+M", 2 }, {"D-M", 19 }, 
    {"M-D", 7 }, {"D&M", 0 }, {"D|M", 21 },
    { NULL, -1 }  // end of list
};

Соответствующий код тоже прост:

static int get_computation_address(const char* cmd) {
    const struct computation* comp = Computations;
    for ( ; comp->text && strcmp(cmd, comp->text) != 0; ++comp) {
    }
    return comp->value;
}

Теперь человеку легко увидеть связанную мнемонику и значения, и явный подсчет не требуется, потому что вместо этого используется контрольное значение.

Упростите создание инструкций

Сейчас очень много звонков to_bin который выполняет всевозможную работу, направляя значения в нужное место для окончательной инструкции. Вместо этого, почему бы просто не вычислить в двоичном формате и использовать побитовую операцию или для сборки инструкции? Например, если у нас есть значения для a, comp, dest а также jmp, мы можем легко построить C-инструкцию:

int inst = 0xE000 | (a << 12) | (comp << 6) | (dest << 3) | jmp;

Даже лучше, поскольку значения a а также comp являются неявными из мнемоники, мы могли бы использовать структуру таблицы выше, чтобы содержать и то, и другое, и поместить их в таблицу как значение, которое нам понадобится уже сдвинут на место. Итак, таблица будет такой:

const struct computation {
    const char *text;
    int value;
} Computations[] = {
    { "0", 42<<6 }, { "1", 63<<6 }, { "-1", 58<<6 }, { "D", 12<<6 }, { "A", 48<<6 }, 
    { "!D", 13<<6 }, { "!A", 49<<6 }, { "-D", 15<<6 }, { "-A", 51<<6 }, {"D+1", 31<<6 }, 
    {"A+1", 55<<6 }, {"D-1", 14<<6 }, {"A-1", 50<<6 }, {"D+A", 2<<6 }, {"D-A", 19<<6 },
    { "A-D", 7<<6 }, {"D&A", 0<<6 }, {"D|A", (64|21)<<6 }, {"M", (64|48)<<6 }, {"!M", (64|49)<<6 }, 
    {"-M", (64|51)<<6 }, {"M+1", (64|55)<<6 }, { "M-1", (64|50)<<6 }, {"D+M", (64|2)<<6 }, {"D-M", (64|19)<<6 }, 
    {"M-D", (64|7)<<6 }, {"D&M", (64|0)<<6 }, {"D|M", (64|21)<<6 },
    { NULL, -1 }  // end of list
};

Теперь мы можем получить значение прямо отсюда, используя | для составления инструкции в двоичном формате и однократного преобразования в строку ASCII.

Упростите преобразование в ASCII

Небольшой простой способ преобразовать 16-битный int в двоичный файл выглядит так:

for (int mask = 0x8000; mask; mask >>= 1) {
    *dst++ = (mask & instruction) ? '1' : '0';
}

Избегайте копирования строк

в parse вместо синтаксического анализа временной строки и последующего копирования оттуда в место назначения, почему бы просто не поместить ее прямо на место? Вот перезапись, показывающая, как это может выглядеть:

void parse(char* dst, const char* src, HashMap* hash_map) {
    static char label_buffer[256];
    char *current_label = label_buffer;

    for (bool working = true ; working; ++src) {
        if (*src == 'n' || *src == '') {
            // We've either reached a line break or EOF
            *current_label="";
            // Parse the current command
            parse_command(dst, label_buffer, hash_map);
            dst += WORD_LENGTH;
            // Reset label posiion and add n or  to output
            current_label = label_buffer;
            *dst++ = *src;
            working = *src;
        } else {
            *current_label++ = *src;
        }
    }
    // Done, make sure to end with null terminator
    *dst="";
}

Альтернативы Valgrind для проверки распределения памяти

Я не верю, что Valgrind поддерживается на машинах Mac M1, поэтому распределение и освобождение памяти было написано несколько слепым.

К счастью, есть и другие способы проверить, есть ли у вас утечки памяти. В частности, Clang включает поддержку Адрес. Скомпилируйте и свяжите свою программу с fsanitize=address, а затем он распечатает любые утечки и другие потенциальные проблемы во время выполнения.

Избегайте ненужных предварительных деклараций

Вы можете избежать прямого объявления, изменив порядок, в котором функции определены в ваших исходных файлах. Таким образом, вам не придется повторяться, и если вы измените имя или параметры функции, вам нужно будет сделать это только в одном месте.

Сообщение об ошибках

Приятно иметь одну функцию, которая выводит ошибку и выходит из программы с ненулевым кодом выхода. Однако есть несколько проблем с вашей реализацией.

Во-первых, убедитесь, что вы пишете сообщения об ошибках в stderr. Это гарантирует, что они не будут смешиваться с обычным выводом, записанным в stdout, что особенно важно, если вывод перенаправлен. Кроме того, я рекомендую вам использовать EXIT_FAILURE как код выхода для всех ошибок. Остальные значения нестандартны.

Кроме того, необходимость поддерживать список перечислений и соответствующих им сообщений об ошибках — это небольшая работа, и это не очень гибко. Я рекомендую вам либо использовать функцию BSD err() или переопределите его самостоятельно. Таким образом, вы можете сообщать об ошибках, например:

if (size_in_bytes > MAX_ASSEMBLED_SIZE / 2) {
    err(EXIT_FAILURE, "File too large, make sure it is less than %d bytes", MAX_ASSEMBLED_SIZE / 2);
}

Подумайте о том, чтобы разрешить ввод от stdin и вывод в stdout

Вместо того, чтобы всегда читать из файла и записывать результат в другой файл, рассмотрите возможность stdin а также stdout использоваться. Это позволяет использовать ваш ассемблер в более сложных командных строках, где, например, сборка создается одной программой и затем может быть передана непосредственно в ваш ассемблер. Это вполне нормально для любой UNIX-подобной операционной системы, но даже Windows поддерживает такое перенаправление ввода и вывода.

В частности, если программа вызывается с нулевыми аргументами, считайте из stdin и напишите stdout. Если он вызывается с одним аргументом, считайте из предоставленного имени файла и напишите в stdout.

Также убедитесь, что вы не распечатываете диагностику на stdout где он мог смешаться с выходом сборки. Использовать stderr для любых обновлений прогресса, или рассмотрите возможность просто не печатать никакой информации, если все пойдет хорошо.

Ненужное ограничение размера файла?

Я вижу, что машинный язык Hack поддерживает 32-битные указатели, поэтому он должен иметь возможность адресовать до 4 ГБ. Я думаю, что причина, по которой вы ограничиваете максимальный размер сборки до 1 мегабайта, заключается в том, что вы выделяете потенциально очень большие массивы в стеке в assemble(). Использовать malloc() для выделения этих массивов вместо этого, чтобы вы не исчерпали ограниченное пространство стека.

Предоставлять правильные сообщения об ошибках

Если malloc() сбой, ваша программа в настоящее время выводит сообщение об ошибке «Не удалось прочитать файл», что вводит в заблуждение. Убедитесь, что сообщения об ошибках четко описывают реальную произошедшую ошибку.

Избегайте произвольных ограничений для имен файлов

Вы объявляете строку output_file_name как массив из 256 charс. Но имена файлов могут быть длиннее. Не предполагайте никаких ограничений. Либо используйте strdup() (что, к сожалению, не является стандартным C пока что), или malloc(strlen(...) + 1), или, что еще лучше, вообще ничего не выделять:

FILE *output = fopen(output_name ? output_name : "assembled.hack", "w");

Отсутствует проверка ошибок

Ошибки ввода-вывода могут произойти в любое время, а не только при открытии файла. Проверить возвращаемое значение fread(), fwrite() а также fclose(), и не забудьте сообщить об ошибке stderr и выйдите из программы с помощью EXIT_FAILURE на случай ошибки.

Кроме того, есть несколько обращений к malloc() в коде, где возвращаемое значение не проверяется.

Уменьшение количества проходов над входом

Вы обрабатываете ввод несколько раз, каждый раз делая что-то одно, например, удаляя комментарии. Это требует времени, и вы также используете много временного хранилища для каждого прохода. В идеале вы должны анализировать все за один проход.

Разбор ввода за один проход можно выполнить, изменив учет меток. Разберите каждую строку, удалите пробелы и комментарии. Если что-то осталось, проверьте, является ли это переменной или меткой. Если это ссылка на метку, которую вы видели ранее, вы можете сразу же заменить ее номером строки. В противном случае вы должны помнить, что вам нужно вставить сюда номер строки, когда вы дойдете до строки, где определена метка. Когда вы встретите определение метки, проверьте, нужно ли вам исправить какие-либо номера строк в более раннем выводе.

Именование вещей

«Stripper.c» может быть не лучшим выбором для имени файла, особенно если он делает больше, чем просто удаляет вещи: он также имеет дело с переменными.

Я думаю, вы отлично поработали с именами функций и переменных. Некоторые мелкие проблемы:

• dst в save_variables() вероятно, следует переименовать в src, и быть сделано const пока вы на нем.
• В предварительном заявлении error_string_for_code(), star должно быть str.

Правильное использование tolower() а также isdigit()

Функции из <ctype.h> ожидать int в качестве аргумента, а не char. Кроме того, они ожидают, что значение будет больше или равно нулю для всех допустимых символов. Правильный способ их использования — использовать char к unsigned char прежде чем использовать его в качестве аргумента, например:

if (!isdigit((unsigned char)str[i])) {
    return false;
}

Предпочитать strtol() над atoi()

Использовать strtol() вместо того atoi()и используйте второй аргумент, чтобы убедиться, что все число было прочитано правильно.

Лучший способ передать размеры в malloc()

Вместо того:

Node *new = malloc(sizeof(Node));

Обычно рекомендуется писать:

Node *new = malloc(sizeof *new);

Это позволяет избежать повторения типа дважды. Более того, sizeof(char) по стандарту C гарантированно равняется 1, поэтому вместо:

new->key = malloc(sizeof(char) * (strlen(key) + 1));

Вы можете просто написать:

new->key = malloc(strlen(key) + 1);

Утечка памяти при освобождении хеш-карты

hash_map_free() освобождает только массив указателей на сегменты, но не содержимое самих сегментов.