Skip to content

Conversation

@Physic69
Copy link
Contributor

No description provided.

@Physic69 Physic69 changed the title Add signal functions (allowed, blocked, pending) to lualinux Add signal functions (modify, pending) Dec 27, 2025
@Physic69 Physic69 force-pushed the idk branch 2 times, most recently from 92398d0 to 8f66707 Compare December 31, 2025 20:17
lib/lualinux.c Outdated
sigaddset(&newmask, signum);

lunatik_try(L, sigprocmask, cmd, &newmask, NULL);
lua_pushboolean(L, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

why pushboolean here?

lib/lualinux.c Outdated
SIGSTATE_ALLOWED,
};

static const char *const sigstate_opts[] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

it shouldn't be static, right?

lib/lualinux.c Outdated
[SIGSTATE_BLOCKED] = "blocked",
[SIGSTATE_PENDING] = "pending",
[SIGSTATE_ALLOWED] = "allowed",
NULL
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
NULL
NULL,

Copy link
Contributor

Choose a reason for hiding this comment

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

actually, we don't need this NULL here, right?

};

int signum = luaL_checkinteger(L, 1);
enum sigstate_cmd cmd = (enum sigstate_cmd)luaL_checkoption(L, 2, "blocked", sigstate_opts);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we should have a default = blocked here.. but I will leave this decision to you ;-).. what would be the better ergonomics for this API..

Copy link
Contributor Author

@Physic69 Physic69 Jan 4, 2026

Choose a reason for hiding this comment

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

i feel like its better than having pending at the moment

Copy link
Contributor

Choose a reason for hiding this comment

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

thing is.. we don't need to have any default.. but it's alright to me.. =)

Copy link
Contributor

@lneto lneto left a comment

Choose a reason for hiding this comment

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

it's looking good to me.. just small comments..

@lneto lneto marked this pull request as ready for review January 4, 2026 21:16
@lneto lneto merged commit 29e8e40 into luainkernel:master Jan 4, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants