Skip to content

Fix bugs in python sixel_encoder_encode_bytes#223

Merged
saitoha merged 1 commit intosaitoha:masterfrom
xyzzy42:python-fixes
Apr 14, 2026
Merged

Fix bugs in python sixel_encoder_encode_bytes#223
saitoha merged 1 commit intosaitoha:masterfrom
xyzzy42:python-fixes

Conversation

@xyzzy42
Copy link
Copy Markdown
Contributor

@xyzzy42 xyzzy42 commented Apr 13, 2026

Data length incorrect error message uses buf.len, but this does not exist, correct function is len(buf), as used on previous line.

Argument type check via ctype was setting args to the wrong function from libsixel.

The palette length when no palette is specified was passed as None, it should be 0. This error is detected after argument type checking is fixed.

cbuf is created from buf argument, but it's not done correctly. What it does is make a pointer the address made from the first 32/64 bits of data in the buffer. I.e., it makes a pointing using the image pixels as if they were a pointer. That makes a random pointer to nowhere.

But cbuf is never used, so this code can just be deleted. ctypes will convert a bytes() object or various other things into the correct type without help.

Data length incorrect error message uses buf.len, but this does not exist,
correct function is len(buf), as used on previous line.

Argument type check via ctype was setting args to the wrong function from
libsixel.

The palette length when no palette is specified was passed as None, it
should be 0.  This error is detected after argument type checking is fixed.

cbuf is created from buf argument, but it's not done correctly.  What it
does is make a pointer the address made from the first 32/64 bits of data
in the buffer.  I.e., it makes a pointing using the image pixels as if they
were a pointer.  That makes a random pointer to nowhere.

But cbuf is never used, so this code can just be deleted.  ctypes will
convert a bytes() object or various other things into the correct type
without help.
@saitoha
Copy link
Copy Markdown
Owner

saitoha commented Apr 14, 2026

@xyzzy42 Thank you.
It looks like there were quite a few issues in a commit (681adde, 681adde) from about ten years ago.

  • A reference to non-existent buf.len
  • argtypes being set on the wrong libsixel function
  • Assigning None instead of 0 when no palette is specified
  • An unused variable, cbuf

I’m not sure why these mistakes happened, but all of your points are correct, and your code looks good.
The master branch currently does not have infrastructure to test the Python bindings, and we plan to add regression tests on the develop branch.

This fix will be merged into the next version, 1.8.7-r1.

@saitoha saitoha merged commit 078f688 into saitoha:master Apr 14, 2026
2 of 3 checks passed
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