vstinner / hachoir

Hachoir is a Python library to view and edit a binary stream field by field
http://hachoir.readthedocs.io/
GNU General Public License v2.0
604 stars 70 forks source link

Hachoir.editor: Allow writing unaligned bitstreams #54

Closed SwiftPengu closed 4 years ago

SwiftPengu commented 4 years ago

The current implementation sometimes attempts to write a set of bytes (which is more efficient) when the input size is a multiple of 8. However, when the output stream is not aligned to bytes (e.g. because some bits have been written), this causes a crash in hachoir/editor/output.py.

This PR does the following:

  1. Detect when the target address is not byte-aligned, and request bits to be written if found true.
  2. Disable the byte-writing optimization altogether.

The last item could be considered a work-around (what I'm trying to do works with this PR). However, when updating checks all along the regular implementation path (which usually involves checking whether output._bit_pos % 8 != 0, otherwise we eventually end up in OutputStream.writeBytes on line 143, which raises a NotImplementedError) caused editor.EditableFieldSet to request writing more than 256 bits (which fails the assertion in output.py on line 117). I tried working around that by writing 256 bits at a time, but that slowed down the parser to a crawl.

In my opinion, this PR provides a balance by disabling an optimization, while still allowing efficiently copying of both bit and byte-aligned fields. All the while avoiding to implement bit-aligned writing of bytes.

However, you may not want to disable that optimization for your use-cases. In that case, I would recommend only keeping the bugfix in field.py.

vstinner commented 4 years ago

Would it be possible to write a test for this change?

SwiftPengu commented 4 years ago

Added a test.

SwiftPengu commented 4 years ago

Hum, address should also be tested, no? See the first test of copyBytesFrom().

You are right, with the current two tests, there is no particular guarantee that the bitstreams will be aligned. Uploading a commit.

vstinner commented 4 years ago

Merged, thanks.