Skip to content

ext/gmp: gmp_fact() reject values larger than unsigned long. - #21794

Closed
devnexen wants to merge 1 commit into
php:masterfrom
devnexen:gmp_fix_1
Closed

ext/gmp: gmp_fact() reject values larger than unsigned long.#21794
devnexen wants to merge 1 commit into
php:masterfrom
devnexen:gmp_fix_1

Conversation

@devnexen

Copy link
Copy Markdown
Member

No description provided.

@devnexen
devnexen marked this pull request as ready for review April 18, 2026 06:54
@devnexen
devnexen requested a review from Girgias as a code ownerApril 18, 2026 06:54
Comment threadext/gmp/gmp.c
}

// TODO: Check that we don't an int that is larger than an unsigned long?
// Could use mpz_fits_slong_p() if we revert to using mpz_get_si()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This comment shouldn't be deleted (?)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

The TODO is resolved by the new check right below — mpz_fits_ulong_p(gmpnum) is exactly what it was asking for. Keeping the comment would misleadingly suggest the
work is still pending. But wiating fairly Gina's opinion on it first :)

@GirgiasGirgias left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks correct to me, I assume that MPZ function has been available for ages too?

@devnexen

Copy link
Copy Markdown
MemberAuthor

Looks correct to me, I assume that MPZ function has been available for ages too?

yes you re right :)

Sign up for freeto 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

@devnexen@Girgias@LamentXU123