C: Check whether memory allocation was successful

[Garfield96]

What problem is this PR intended to solve?

The C extension didn't check whether memory allocations were successful (= malloc/calloc return a value other than NULL). In the rare case, that memory allocation fails, this leads to undefined behavior, which usually results in a segmentation fault.

This problem was discovered using the static analyzer (-fanalyzer) of GCC.

Have you included adequate test coverage?

There is no easy way to test this behavior, hence I omitted test cases.

Does this change affect the behavior of either the C or the Java implementations?

If no more memory is available rb_eNoMemError is raised. As far as I can tell, it is not necessary to change the Java implementation, since Java would automatically raise an OutOfMemoryError exception.

[larskanis]

Wouldn't it make sense to use ruby_xmalloc instead?

[Garfield96]

Good idea. I don't have much experience with the Ruby API and wasn't aware of this function. I'll change it.

[flavorjones]

+1 we should have been using ruby_xmalloc all along (and in fact this is what we configure libxml2 to use internally).

[flavorjones]

@Garfield96 Thanks so much for submitting this PR. Are you comfortable making the change that Lars suggested? If not, that's OK and I'm happy to do it.

Please also note there exists a ruby_xcalloc that would replace calloc.

[Garfield96]

I started to implement it with ruby allocators and I'll push the new version tomorrow.

[Garfield96]

Ruby allocators are now used everywhere, except for vasprintf. I tried to adapt the vasprintf implementation in nokogiri.c but it results in a segmentation fault. To not block this PR, I'll open another PR for vasprintf.

[flavorjones]

Thanks! I'm on vacation for a few days but will take a look as soon as I can.

…

On Sat, Mar 19, 2022, 8:32 AM Christian Menges ***@***.***> wrote: Ruby allocators are now used everywhere, except for vasprintf. I tried to adapt the vasprintf implementation in nokogiri.c but it results in a segmentation fault. To not block this PR, I'll open another PR for vasprintf. — Reply to this email directly, view it on GitHub <#2480 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAACADYBB5CH6SFDYPJNBU3VAXCMRANCNFSM5RBTP5SA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you commented.Message ID: ***@***.***>

[flavorjones]

I pushed a second commit that uses ruby_xfree to free the nokogiriXsltStylesheetTuple struct in xslt_stylesheet.c, which I think you missed. Otherwise this looks good to me.

I've pinged @larskanis for his review as well, because he's very familiar with memory handling in C extensions.

[flavorjones]

Appveyor is green, not sure why that's not reflected here in the PR checks. I'll give Lars a chance to comment, but I'm pretty confident this is good to merge so I'm planning to do that this weekend. Thank you!

[flavorjones]

Merged! Thank you for your contribution, @Garfield96. I'll credit you in the CHANGELOG.