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

uint32_t are cast as int through spank:get_item(...) #26

Open
zacharyrs opened this issue Dec 2, 2022 · 2 comments
Open

uint32_t are cast as int through spank:get_item(...) #26

zacharyrs opened this issue Dec 2, 2022 · 2 comments

Comments

@zacharyrs
Copy link

When using spank:get_item(...), the returns are sometimes weird negative numbers due to casting from unsigned to signed - I discovered this when getting S_JOB_STEPID.

The following variables are stored as uint32_t:

  • S_JOB_ID
  • S_JOB_STEPID
  • S_JOB_NNODES
  • S_JOB_NODEID
  • S_JOB_LOCAL_TASK_COUNT
  • S_JOB_TOTAL_TASK_COUNT
  • S_TASK_GLOBAL_ID
  • S_STEP_CPUS_PER_TASK

Also, S_JOB_NCPUS is a unit16_t, but this won't be affected by the cast.

It might be nice also to include the magic constants for step IDs - https://github.com/SchedMD/slurm/blob/slurm-22-05-3-1/slurm/slurm.h#LL147-L155.

@kcgthb
Copy link
Member

kcgthb commented Dec 2, 2022

@zacharyrs very good points!
Would you mind submitting a PR for those changes?

@zacharyrs
Copy link
Author

zacharyrs commented Dec 2, 2022

@kcgthb
Would you mind submitting a PR for those changes?

Hah, my hacky fix was to just swap int to int64_t on line 215.

I'm not sure if this is quite appropriate though - it looks like lua can use different sizes inernally that might be an issue depending on where it's compiled.
For reference, I'm pretty sure the default number type in lua is stored as a double, but it looks like that can change too.

I think there are actually a few other issues too - I think memory is stored as uin64_t and this casts to long, which may or may not be signed 64-bit integer, so we'll have the same overflow.

There's a lua_pushinteger method that might be more appropriate too - and I think uses either long or long long, again depending on compilation.

I'm honestly neither a c nor lua programmer, so I'm not too sure what the best approaches are...
I don't want to hit the good old "oh it works on my machine"!

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

No branches or pull requests

2 participants