Opened 5 years ago

Closed 4 years ago

#2581 closed defect (fixed)

Fix Python ctypes conversion for stat64 struct on GNU/Hurd

Reported by: Bas Couwenberg Owned by: grass-dev@…
Priority: normal Milestone: 7.0.2
Component: Python ctypes Version: svn-releasebranch70
Keywords: python Cc:
CPU: Unspecified Platform: All

Description

The Debian package build of GRASS 7.0.0RC1 on the hurd-i386 architecture revealed some more issues.

The attached patch encloses the Python ternary expression in parenthesis to fix an issue with the C to Python conversion of the stat & stat64 structs on GNU/Hurd.

The structs define the final member conditionally, for the stat64 structure this is:

#define _SPARE_SIZE     ((sizeof (__fsid_t) == sizeof (int)) ? 9 : 8)
    int st_spare[_SPARE_SIZE];  /* Room for future expansion.  */
#undef _SPARE_SIZE

This gets converted by ctypesgen to:

('st_spare', c_int * (sizeof(__fsid_t) == sizeof(c_int)) and 9 or 8),

Which causes a TypeError for every Python script including the generated gis.py:

TypeError: second item in _fields_ tuple (index 17) must be a C type

Enclosing the Python expression in parenthesis fixes the error:

('st_spare', c_int * ((sizeof(__fsid_t) == sizeof(c_int)) and 9 or 8)),

Since ctypesgen doesn't look actively maintained upstream anymore, I'm forwarding this patch for inclusion in GRASS only.

Attachments (2)

python-ctypes-ternary.patch (1.2 KB) - added by Bas Couwenberg 5 years ago.
python-ctypes-ternary.2.patch (2.0 KB) - added by Bas Couwenberg 5 years ago.
Also use if/else instead of and/or

Download all attachments as: .zip

Change History (9)

Changed 5 years ago by Bas Couwenberg

Attachment: python-ctypes-ternary.patch added

comment:1 Changed 5 years ago by Bas Couwenberg

Platform: UnspecifiedOther Unix

comment:2 in reply to:  description Changed 5 years ago by glynn

Replying to sebastic:

The attached patch encloses the Python ternary expression in parenthesis to fix an issue with the C to Python conversion of the stat & stat64 structs on GNU/Hurd.

It would be better to use Python's own conditional-expression syntax (a if cond else b). The and-or hack produces the wrong answer if the if-true expression evaluates to false.

Python's conditional-expression syntax probably didn't exist when ctypesgen was written (it was added in 2.5), but GRASS doesn't support any Python version old enough for that to matter (IIRC, we officially require at least 2.6, and having 2.7-isms slip into the code by accident hasn't been particularly uncommon).

Changed 5 years ago by Bas Couwenberg

Also use if/else instead of and/or

comment:3 Changed 5 years ago by Bas Couwenberg

Replying to glynn:

It would be better to use Python's own conditional-expression syntax (a if cond else b). The and-or hack produces the wrong answer if the if-true expression evaluates to false.

Yes, I'm aware of the recommendation to use if/else instead of and/or in Python in place of the ?: ternary operator in C.

I kept the patch as minimal as possible to only fix the nesting issue. I've now updated the patch to also use if/else instead of and/or.

comment:4 Changed 4 years ago by neteler

CPU: x86-32Unspecified
Keywords: python added
Milestone: 7.0.07.0.2
Platform: Other UnixAll

Shall we apply this revised patch?

comment:5 in reply to:  4 ; Changed 4 years ago by glynn

Replying to neteler:

Shall we apply this revised patch?

I think so.

comment:6 in reply to:  5 ; Changed 4 years ago by martinl

Replying to glynn:

Replying to neteler:

Shall we apply this revised patch?

I think so.

done in r65968. If no objection I will backport it to relbr70.

comment:7 in reply to:  6 Changed 4 years ago by martinl

Resolution: fixed
Status: newclosed

Replying to martinl:

done in r65968. If no objection I will backport it to relbr70.

backported in r66031. Closing the ticket.

Note: See TracTickets for help on using tickets.