Memory allocation failures not handled

[infradig]

The code tries but fails (no checking in many places) to detect and pass up allocation failures. As an interim I have put in place setting errno to ENOMEM which works well (so far) for my application.

include

static mp_digit s_alloc(mp_size num) { errno = 0; mp_digit out = malloc(num * sizeof(mp_digit)); //assert(out != NULL); if (out == NULL) errno = ENOMEM;

if DEBUG

for (mp_size ix = 0; ix < num; ++ix) out[ix] = fill;

endif

return out; }

[creachadair]

Thanks for the report. I intended that all the paths that allocate should report failure correctly, but it's entirely possible I missed some.

Workaround aside, if you can point out any specific places you've noticed where I missed a case, I'd appreciate it. It's a little trickier than it seems to tell, because temp variables are set up with a macro (which should check, but isn't obvious) and many initializations do not allocate (they use the internal digit directly).

Nonetheless I don't doubt there may be such cases, and I'd be glad to fix them.

[infradig]

I noticed the macros. I borrowed some ideas from there.

First off the assert in s_alloc() is a no-no to me. A library should never deliberately cause an abort. I figure it wasn't left there intentionally.

Possibly s_lsqr() is worth looking at. I was there when I decided to go the errno route as a quick fix.

Generally though there is much to admire in the library. I really like the single file.

On Tue, Mar 1, 2022 at 1:14 PM M. J. Fromberger @.***> wrote:

Thanks for the report. I intended that all the paths that allocate should report failure correctly, but it's entirely possible I missed some.

Workaround aside, if you can point out any specific places you've noticed where I missed a case, I'd appreciate it. It's a little trickier than it seems to tell, because temp variables are set up with a macro (which should check, but isn't obvious) and many initializations do not allocate (they use the internal digit directly).

Nonetheless I don't doubt there may be such cases, and I'd be glad to fix them.

— Reply to this email directly, view it on GitHub https://github.com/creachadair/imath/issues/54#issuecomment-1054954664, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFNKSER7UPBEM3NRTASKXV3U5Q2Z5ANCNFSM5PSL7BDQ . You are receiving this because you authored the thread.Message ID: @.***>

[creachadair]

The assert was not accidental, but a production build should generally define NDEBUG to disable assertions anyway. Having them around for debug builds is convenient though.

[infradig]

I get your point about NDEBUG but I work on a language interpreter and do a lot in debug mode. One of the features of the language is catching error conditions, one of which is an out-of-memory error. I often work with a low ulimit set to make this easier (and common) to pick up. But it's no big hassle.

On Tue, Mar 1, 2022 at 1:46 PM M. J. Fromberger @.***> wrote:

The assert was not accidental, but a production build should generally define NDEBUG to disable assertions anyway. Having them around for debug builds is convenient though.

— Reply to this email directly, view it on GitHub https://github.com/creachadair/imath/issues/54#issuecomment-1054974938, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFNKSESRNJY7GVPUDL77U2DU5Q6RXANCNFSM5PSL7BDQ . You are receiving this because you authored the thread.Message ID: @.***>

[creachadair]

I get your point about NDEBUG but I work on a language interpreter and do a lot in debug mode. One of the features of the language is catching error conditions, one of which is an out-of-memory error. I often work with a low ulimit set to make this easier (and common) to pick up. But it's no big hassle.

I wouldn't be opposed to dropping the assert from the allocation helper. I think that is the only case that isn't checking a validity property of the interface. (That said—it's also easy enough to just delete it, as you've done in your build).