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

unpackdat: Update algorithm #8

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

Conversation

wosk
Copy link

@wosk wosk commented Apr 19, 2022

  • recognize two chunks formats
  • Trim gap before first chunk
  • Print dat header

You have to investigate chunk offsets before cut bin file

* recognize two chunks formats
* Trim gap before first chunk
* Print dat header

You have to investigate chunk offsets before cut bin file
@fenugrec
Copy link
Owner

Nice ! Never expected to get pull reqs on this project...
Do you have a link to an example .dat that uses the new chunk format you added ?

Copy link
Owner

@fenugrec fenugrec left a comment

Choose a reason for hiding this comment

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

The main issues are :

  • this will break 9HA7D.dat with a weird 0x70 byte header (attached)
  • casting raw buffers to a struct is looking for trouble : packing and endianness issues.

9HA7D.zip

@@ -16,15 +19,60 @@

FILE *dbg_stream;

#define FMT_CHUNK_EXT 0x80
#define CRC_TAIL_LEN 3
#define MAX_DAT_OFFSET 0xffffff
Copy link
Owner

Choose a reason for hiding this comment

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

I assume MAX_DAT_OFFSET is to cover the maximum "AH:AM:AL" value ?

rf->dbuf = malloc(file_len);
// Allocate max size for output according to offset field dimension
rf->dbuf_size = MAX_DAT_OFFSET;
rf->dbuf = malloc(rf->dbuf_size);
Copy link
Owner

Choose a reason for hiding this comment

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

Ok, but maybe the size should be +1 : if the last chunk has len=0x80 starting at 0xff ff80 then the last byte would overflow.


char ver_str[6]; // e.g. LBC ZE1/ VCM

uint32_t off17; // 00 00 00 80
Copy link
Owner

Choose a reason for hiding this comment

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

If I count properly, this field is not 4-byte aligned in the (packed) struct, correct ?

printf("\n");
}

void unpack_dat(FILE *outf, struct datfile *rf, bool trim) {
Copy link
Owner

@fenugrec fenugrec Apr 20, 2022

Choose a reason for hiding this comment

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

yes, passing in a struct * is good
Can you add a comment for argument "trim"

temp_addr >>= 8;
if (temp_addr != (curaddr + DCHUNK_DLEN)) {
printf("addr skip @ %lX\n", (unsigned long) temp_addr);
const dat_header_t *dath = (const dat_header_t *)src;
Copy link
Owner

Choose a reason for hiding this comment

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

I don't like casting a raw buffer to a struct like this, especially if the struct has unaligned bytes. It also leads to assuming host endianness.
Either at least memcpy to your struct (and mark the struct with __attribute packed etc), or copy the elements one by one with reconst_XX() as required.

const chunk_t *ch = NULL;
if (ch_header->fmt == FMT_CHUNK_EXT) {
// 80 SRC TGT LEN 34 82
ch = (chunk_t*)((uint8_t*)ch_header + sizeof(chunk_header_t));
Copy link
Owner

Choose a reason for hiding this comment

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

casting to struct

}
// TODO check whether ch buffer is not go beyond the end of file

if (ch->cmd != 0x8234) {
Copy link
Owner

Choose a reason for hiding this comment

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

Here, you're assuming host endianness. Why not memcmp() with pm[] ?

break;
}

offset = ch->offset_hi << 16 | ch->offset_mid << 8 | ch->offset_low;
Copy link
Owner

Choose a reason for hiding this comment

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

I would prefer either using a truncated reconst_32() call like I did, or better, adding a reconst_24() to nislib.c+h

dest = rf->dbuf + offset;
memcpy(dest, ch->payload, chklen);

ch_header = (const chunk_header_t *)((uint8_t*)ch + offsetof(chunk_t, payload) + chklen + CRC_TAIL_LEN);
Copy link
Owner

Choose a reason for hiding this comment

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

casting to struct


if (argc !=3) {
printf("%s <file.dat> <out.dat> : unpack payload to <out.dat> etc\n",argv[0]);
if (argc < 3) {
Copy link
Owner

Choose a reason for hiding this comment

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

what was wrong with !=3 ?

@wosk wosk closed this Apr 25, 2022
@wosk wosk reopened this Apr 25, 2022
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