Skip to content

Use int32_t where Python passes an int32 array - #65

Merged
HazenBabcock merged 2 commits into
ZhuangLab:masterfrom
HazenBabcock:fix-int32-abi
Sep 8, 2026
Merged

HazenBabcock merged 2 commits into
ZhuangLab:masterfrom
HazenBabcock:fix-int32-abi

Conversation

@HazenBabcock

Copy link
Copy Markdown
Member

Fixes #27.

The issue asked for int32_t instead of int "in many places". The places
that matter turn out to be identifiable rather than a matter of taste, and
there are ten of them, not the 130 that a raw count of int * suggests.

C-to-C calls do not care what sizeof(int) is — the compiler is
self-consistent — and a ctypes.c_int scalar matches a C int at any size.
The assumption only bites where Python hands C a numpy int32 buffer and C
reinterprets it as int[], because ctypes checks the dtype on the Python
side only. A mismatch there is silent memory corruption, not an error.

Scanning every *_c.py argtypes block against the C declarations, Python
passes an int32 array to 24 functions. 14 already used int32_t, all of
them in multi_fit.c and ia_utilities.c — the core was converted at some
point and the rest was not, which is why the tree currently holds two
conventions with no way to tell which is intended.

19199451 converts the remaining ten

file function parameters
frc/frc.c calc_frc frc_counts
dbscan/dbscan.c dbscan db_cat, db_l
locClSize cl_counts, cl_id
recategorize cl_id, category
L1H/homotopy_general.c getVisited vis
L1H/homotopy_imagea_common.c getPeaks peak_c
sa_library/grid.c grid2D grid, i_x, i_y
grid3D grid, i_x, i_y, i_z
sa_library/cs_decon_utilities.c label labels
moments labels

None has a caller in C, so each change is a declaration and a definition with
no ripple. The other 120 int * parameters never cross the ctypes boundary
and are left alone.

Nothing changes today, and provably so. int32_t is a typedef for int
on every platform this package supports, LP64 and LLP64 alike, so compiling
each file before and after gives byte-identical objects — six checksums are
in the commit message. That identity is also why the compiler cannot check
this work: with int32_t and int the same type, a wrong conversion
produces no diagnostic. The check is the scan.

Note there are two functions named getPeaks. The one taking an int32
array is in L1H. decon_storm/mlem_sparse.c has another whose int *n is an
output count passed as byref(c_int()), so it is not a boundary case, and
that file is not in SConstruct in any event. A scan that keys on function
name must report every definition, not the first, or it hides one behind the
other.

849e4fa3 fixes a function that matched nothing

grid.c declared grid3DZInclusive() and documented it under that name,
then defined grid3DInclusize() — declared-without-definition and
defined-without-declaration at once, wrong since 6a486010. Nothing calls
it. Renamed to match the declaration and the comment, and given the same
int32_t treatment as its two siblings so the file does not keep one
function on the old convention.

Verified by symbol table rather than checksum, since a rename changes the
symbol: exactly one line differs, at the same address, and the disassembly is
identical instruction for instruction once that name is normalized.


Full test suite passes, 278 tests. No new compiler warnings.

HazenBabcock and others added 2 commits September 8, 2026 15:20
Fixes ZhuangLab#27, which asked for int32_t instead of int "in many places". The
places that matter are fewer than the phrasing suggests, and they are
identifiable rather than a matter of taste.

C to C calls do not care what sizeof(int) is, since the compiler is
self-consistent, and a ctypes.c_int scalar matches a C int at any size.
The assumption only bites where Python hands C a numpy int32 buffer and
C reinterprets it as int[], because ctypes checks the dtype on the
Python side only. A mismatch there is silent memory corruption, not an
error.

Scanning every *_c.py argtypes block against the C declarations, Python
passes an int32 array to 24 functions. 14 already used int32_t, all of
them in multi_fit.c and ia_utilities.c: the core was converted at some
point and the rest was not, which is why the tree currently holds two
conventions with no way to tell which is intended. This converts the
remaining ten:

    frc/frc.c                        calc_frc      frc_counts
    dbscan/dbscan.c                  dbscan        db_cat, db_l
                                     locClSize     cl_counts, cl_id
                                     recategorize  cl_id, category
    L1H/homotopy_general.c           getVisited    vis
    L1H/homotopy_imagea_common.c     getPeaks      peak_c
    sa_library/grid.c                grid2D        grid, i_x, i_y
                                     grid3D        grid, i_x, i_y, i_z
    sa_library/cs_decon_utilities.c  label         labels
                                     moments       labels

and adds stdint.h to the five files that lacked it, plus to
homotopy_imagea_common.h, which now names int32_t in a declaration and
is included by homotopy_imagea.c, which has no stdint.h of its own.

None of the ten has a caller in C, so each change is a declaration and a
definition with no ripple. The other 120 int* parameters in the tree
never cross the ctypes boundary and are left alone.

Note that there are two functions named getPeaks(). The one taking an
int32 array is in L1H. decon_storm/mlem_sparse.c has another, whose
int *n is an output count passed as byref(c_int()) and bound as
c_void_p, so it is not a boundary case, and that file is not in
SConstruct in any event.

Nothing changes today, and provably so. int32_t is a typedef for int on
every platform this package supports, LP64 and LLP64 alike, so compiling
each file before and after gives byte identical objects:

    frc                     c32457ae2ebeeb8305c9cad368b656f1
    dbscan                  f9900f35d6a712b50692d9b691d23862
    homotopy_general        decba0bc02496737904edeaa9e1a2c94
    homotopy_imagea_common  6c85ec236b7ba856965b41a0c70f9f0e
    grid                    01c7953b01aa98ad064b62127663a2c6
    cs_decon_utilities      3cdb3093f179c5fe6f0aaeb92220d69d

Both versions have to be compiled from the same filename for that to
mean anything, since gcc embeds the source name. No new warnings; the
two fread warnings in homotopy_imagea_common.c are pre-existing and
appear identically before and after.

That identity is also why the compiler cannot check this work: with
int32_t and int the same type, a wrong conversion produces no
diagnostic. The check is the scan, and it has to report every C
definition of a name rather than the first one it finds, which is what
originally hid the L1H getPeaks behind the decon_storm one.

Note that dbscan.c has CRLF line endings, so it was edited in binary and
its line ending count is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
grid.c declared grid3DZInclusive() and documented it under that name,
then defined grid3DInclusize(). So the same function was declared
without a definition and defined without a declaration at once, which is
the pair of defects 413d2c1 and e9ffba5 fixed in multi_fit.h, here in
a single function. Nothing calls it, in C or through ctypes.

Two of the three occurrences say grid3DZInclusive, and Inclusize is not
a word, so the definition is what was wrong. Wrong since 6a48601, the
commit that put the file in its current location.

Also converts its four array parameters to int32_t, matching grid2D()
and grid3D() next to it after 76080b6a. Python does not call this one so
it is not a boundary function today, but it takes the same arguments as
its siblings, and leaving one function in the file on the old convention
would recreate in miniature the split that commit set out to remove.

This one cannot be verified by a byte identical object, since renaming a
function changes its symbol. Instead, compiling before and after gives a
symbol table differing in exactly one line,

    < 0000000000000160 T grid3DInclusize
    > 0000000000000160 T grid3DZInclusive

at the same address, and a disassembly that is identical
instruction for instruction once that one name is normalized.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@HazenBabcock
HazenBabcock merged commit ec9c5f3 into ZhuangLab:master Sep 8, 2026
5 checks passed
@HazenBabcock
HazenBabcock deleted the fix-int32-abi branch September 8, 2026 22:10
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.

C int may not be 4 bytes

1 participant