Skip to content

QA - Test Cov - ext:pcntl - pcntl_signal() - max signal allowed #8956

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

Merged
merged 4 commits into from
Jul 11, 2022

Conversation

juan-morales
Copy link
Contributor

Extension: pcntl
Function: pcntl_signal
Description:
When provided the $signal parameter is greater than allowed in operating system, then expect an error.

I hard-coded the number 1000000 on purpose because I cannot find a constant or function that can give me the max allowed in order to increment it.

Please, all advices are welcome.

I also attach what was covered in the following screenshots:

Before
image

After
image

@cmb69
Copy link
Member

cmb69 commented Jul 8, 2022

signals are of type int, i.e. for our purposes always signed 32bit numbers. So using 2147483648 would definitely work, but not on 32bit architectures (that number would be represented as float there). Anyhow, 1000000 should be large enough for all systems.

Add new line at the end

Co-authored-by: Christoph M. Becker <[email protected]>
@juan-morales
Copy link
Contributor Author

@cmb69 @Girgias can you support me here? How can I see what happened for MAC and FreeBSD so I understand why failed for them?

@Girgias
Copy link
Member

Girgias commented Jul 11, 2022

@cmb69 @Girgias can you support me here? How can I see what happened for MAC and FreeBSD so I understand why failed for them?

Just look at the build log, as the test diff will be shown

@juan-morales
Copy link
Contributor Author

@cmb69 @Girgias can you support me here? How can I see what happened for MAC and FreeBSD so I understand why failed for them?

Just look at the build log, as the test diff will be shown

Thanks. I never notice that.

Change is ready for review again.

@Girgias Girgias merged commit 23654a1 into php:master Jul 11, 2022
@Girgias
Copy link
Member

Girgias commented Jul 11, 2022

Thank you :)

@juan-morales
Copy link
Contributor Author

Thank you :)

To you and @cmb69 ... a lot.

I will continue adding more coverage to our code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants