Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parsing support for Install files. #29

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ChrisKader
Copy link

No description provided.

Copy link
Member

@ferronn-dev ferronn-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a first pass on the data structures, I'll have more to say about the parsing code itself


// Structure to hold the parsed install file information
typedef struct {
int hash_size;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

everything else in tactless assumes a 16-byte hash; more consistent to just do the same. this field doesn't need to exist

// Structure to hold the parsed install file information
typedef struct {
int hash_size;
int version;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should just be asserted to some value; doesn't need to be here

int hash_size;
int version;
int num_tags;
InstallTagEntry tags[MAX_TAGS];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be dynamically allocated, and just declared InstallTagEntry *tags here

int num_tags;
InstallTagEntry tags[MAX_TAGS];
int num_entries;
InstallFileEntry files[MAX_FILES];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

@@ -518,6 +520,227 @@ static int download_build_config(CURL *curl, const struct cdns *cdns,
return ret;
}

#define MAX_TAGS 128
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove all three of these and use dynamic allocation instead

#define MAX_FILE_NAME_LENGTH 255

// Structure to hold an install tag entry
typedef struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the typedef, and use snake case: struct install_tag_entry {

uint8_t *files;
} InstallTagEntry;

// Structure to hold an install file entry
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same deal here: remove comment, remove typedef, snake case

uint8_t *tags;
} InstallFileEntry;

// Structure to hold the parsed install file information
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here re comment, typedef, name

InstallFileEntry files[MAX_FILES];
} InstallFile;

void print_install_file(const InstallFile *install) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see how the various _dump functions are set up, exposed in tactless.h, and invoked from main.c

printf("\n");
}

int parse_install_file(const uint8_t *data, size_t data_size,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rename it parse_install for consistency

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants