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

Fix Issue #548 #547

Merged
merged 2 commits into from
Apr 22, 2016
Merged

Fix Issue #548 #547

merged 2 commits into from
Apr 22, 2016

Conversation

kdexd
Copy link

@kdexd kdexd commented Apr 22, 2016

This PR is a fix of issue #548 as well as a simplification of PR #540. Previous implementation caused ocassional segfaults in subsequent test runs. Here, the complete file enum.py has been removed and corresponding variables from C enums have been defined as macros in struct.py. This ensures that the code is still readable as before.

- These macros have same names as defined in
  tardis/montecarlo/src/status.h
@@ -22,7 +21,7 @@ class RPacket(Structure):
('d_boundary', c_double),
('d_cont', c_double),
('next_shell_id', c_int64),
('status', RPacketStatus),
('status', c_int),
Copy link
Contributor

@yeganer yeganer Apr 22, 2016

Choose a reason for hiding this comment

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

Use c_status_t here and define it as c_status_t = c_int above.
Or c_packet_status_t. Whatever sounds best.

Copy link
Author

Choose a reason for hiding this comment

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

  • Done. ( c_rpacket_status_t )

@kdexd kdexd changed the title Simplify the way C enums were reflected in Python. Fix Issue #548 Apr 22, 2016
@kdexd kdexd force-pushed the enums-for-cmontecarlo branch from e4a9580 to fd7a862 Compare April 22, 2016 09:07
@kdexd
Copy link
Author

kdexd commented Apr 22, 2016

Great, everything is neat.
@yeganer I believe this resolves #548 and silences down segfaults occuring in #544.

@@ -1,5 +1,8 @@
from ctypes import Structure, POINTER, c_int, c_int64, c_double, c_ulong

c_rpacket_status_t = c_int
c_cont_status_t = c_int

Copy link
Contributor

Choose a reason for hiding this comment

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

you missed c_error_t here

Copy link
Author

Choose a reason for hiding this comment

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

It isn't used in structs right now, though declaration would be helpful.

Copy link
Author

Choose a reason for hiding this comment

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

  • Done. ( c_tardis_error_t )

- C enums can be treated as ints (signed or unsigned)
  so, RPacket.status is mapped as c_rpacket_status_t
  and this, is declared as c_int before.

- Similar workaround for StorageModel.cont_status.

- c_tardis_error_t also defined alongwith these two.
@kdexd kdexd force-pushed the enums-for-cmontecarlo branch from fd7a862 to 5d15f05 Compare April 22, 2016 09:41
@yeganer
Copy link
Contributor

yeganer commented Apr 22, 2016

@tardis-sn/tardis-core Can someone please sign this off? This resolves issue #548.

@wkerzendorf wkerzendorf merged commit bf2fa49 into tardis-sn:master Apr 22, 2016
@kdexd kdexd deleted the enums-for-cmontecarlo branch April 22, 2016 13:15
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.

3 participants