Check slong is used for variables passed as params to _si functions #32

Closed
fredrik-johansson opened this Issue Nov 21, 2013 · 3 comments

Comments

Projects
None yet
2 participants
@fredrik-johansson

See title.

@wbhart wbhart modified the milestone: flint-2.5 Jul 23, 2015

wbhart added a commit that referenced this issue Jul 29, 2015

Fix sparse polynomial O(n^2) creation issue (#32) by zeroing coeffici…
…ents when

set_length/truncate is called, rather than when setting coefficients.
@wbhart

This comment has been minimized.

Show comment
Hide comment
@wbhart

wbhart Jul 30, 2015

Owner

There are five files xaa, xab, xac, xad, xae each with around 72 potential cases to check.

Here are the suspicious cases per file after volunteers examined the files. Each will have to be checked very carefully and/or fixed.

xaa:

No problems to report.

xab:

There is no cast here:
fmpz/cdiv_q_ui.c: fmpz_set_si(f, - (((ulong) -c1) / c2))
There might be more casts as well when defining q et al. checked OK

Same for fmpz/fdiv_q.c and other division files. Also note the
discrepancy in the types used there and in the previous file (most
division files seem to follow this latter file convention). checked OK

Also check fmpz/add.c and fmpz/fdiv_q_ui.c. checked OK

This one looks dangerous:
fmpz/cdiv_q_2exp.c: fmpz_set_si(f, -((-d) >> FLINT_MIN(exp, should be OK
FLINT_BITS - 2)));
I'm not sure what the C standard says about right shifts.

No cast from ulong to slong here (though the result is > 0):
fmpz/multi_CRT_ui.c: fmpz_set_si(output, residues[0] - p); checked OK

In fmpz/xgcd_partial.c a few slong are fed to _ui functions which should be ok. checked OK

In
fmpz/is_prime_morrison.c: fmpz_set_si(B, b); b FIXED
is a mp_limb_t

In
fmpz/add_ui.c: fmpz_set_si(f, x + c checked OK
ulong + slong.

xac:

fmpz/divexact_ui.c: fmpz_set_si(f, c1 / (slong) h); checked OK

fmpz/invmod.c: return (gcd == UWORD(1) ? fmpz_set_si(f, inv), 1 : 0); checked OK
fmpz/invmod.c: return (gcd == UWORD(1) ? fmpz_set_si(f, inv), 1 : 0); checked OK

fmpz.h: fmpz_set_si(f, x - m); checked OK

fmpz_mat/hadamard.c: fmpz_set_si(fmpz_mat_entry(Q, r, c), checked OK

xad:

No problems to report.

xae:

No problems to report.

Owner

wbhart commented Jul 30, 2015

There are five files xaa, xab, xac, xad, xae each with around 72 potential cases to check.

Here are the suspicious cases per file after volunteers examined the files. Each will have to be checked very carefully and/or fixed.

xaa:

No problems to report.

xab:

There is no cast here:
fmpz/cdiv_q_ui.c: fmpz_set_si(f, - (((ulong) -c1) / c2))
There might be more casts as well when defining q et al. checked OK

Same for fmpz/fdiv_q.c and other division files. Also note the
discrepancy in the types used there and in the previous file (most
division files seem to follow this latter file convention). checked OK

Also check fmpz/add.c and fmpz/fdiv_q_ui.c. checked OK

This one looks dangerous:
fmpz/cdiv_q_2exp.c: fmpz_set_si(f, -((-d) >> FLINT_MIN(exp, should be OK
FLINT_BITS - 2)));
I'm not sure what the C standard says about right shifts.

No cast from ulong to slong here (though the result is > 0):
fmpz/multi_CRT_ui.c: fmpz_set_si(output, residues[0] - p); checked OK

In fmpz/xgcd_partial.c a few slong are fed to _ui functions which should be ok. checked OK

In
fmpz/is_prime_morrison.c: fmpz_set_si(B, b); b FIXED
is a mp_limb_t

In
fmpz/add_ui.c: fmpz_set_si(f, x + c checked OK
ulong + slong.

xac:

fmpz/divexact_ui.c: fmpz_set_si(f, c1 / (slong) h); checked OK

fmpz/invmod.c: return (gcd == UWORD(1) ? fmpz_set_si(f, inv), 1 : 0); checked OK
fmpz/invmod.c: return (gcd == UWORD(1) ? fmpz_set_si(f, inv), 1 : 0); checked OK

fmpz.h: fmpz_set_si(f, x - m); checked OK

fmpz_mat/hadamard.c: fmpz_set_si(fmpz_mat_entry(Q, r, c), checked OK

xad:

No problems to report.

xae:

No problems to report.

@fredrik-johansson

This comment has been minimized.

Show comment
Hide comment
@fredrik-johansson

fredrik-johansson Jul 30, 2015

The one in fmpz_mat/hadamard.c: is OK; the argument type is actually int. The longs are safe also and could be changed to either int or slong. In fact all the fmpz values being set in this file are -1, 0, 1.

The one in fmpz_mat/hadamard.c: is OK; the argument type is actually int. The longs are safe also and could be changed to either int or slong. In fact all the fmpz values being set in this file are -1, 0, 1.

@wbhart

This comment has been minimized.

Show comment
Hide comment
@wbhart

wbhart Jul 30, 2015

Owner

Thanks for checking. I may as well set the longs to slongs, just so we
don't keep examining the same thing in future.

On 30 July 2015 at 16:58, Fredrik Johansson notifications@github.com
wrote:

The one in fmpz_mat/hadamard.c: is OK; the argument type is actually int.
The longs are safe also and could be changed to either int or slong. In
fact all the fmpz values being set in this file are -1, 0, 1.


Reply to this email directly or view it on GitHub
#32 (comment).

Owner

wbhart commented Jul 30, 2015

Thanks for checking. I may as well set the longs to slongs, just so we
don't keep examining the same thing in future.

On 30 July 2015 at 16:58, Fredrik Johansson notifications@github.com
wrote:

The one in fmpz_mat/hadamard.c: is OK; the argument type is actually int.
The longs are safe also and could be changed to either int or slong. In
fact all the fmpz values being set in this file are -1, 0, 1.


Reply to this email directly or view it on GitHub
#32 (comment).

@wbhart wbhart closed this Aug 3, 2015

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