rpcgen: make compiler arglist allocation dynamic
ClosedPublic
Actions

Authored by brooks on Oct 10 2019, 4:36 PM.

Details

Reviewers

emaste
arichardson

Commits

rS353939: MFC r353569:
rS353901: MFC r353569:
rS353569: rpcgen: make compiler arglist allocation dynamic

Summary

In CheriBSD we exceed the 19 non-NULL arguments in the static array. Add
a simple size doubling allocator and increase the default to 32.

GC remnants of support for fixed arguments.

Test Plan

Survives a buildworld including some calls to rpcgen.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 26980
Build 25280: arc lint + arc unit

Event Timeline

brooks created this revision.Oct 10 2019, 4:36 PM

Harbormaster completed remote builds in B26980: Diff 63128.Oct 10 2019, 4:37 PM

I wish the C standard library had some kind of dynamic array so that this is not reimplement in every program...

I wonder if we could use something like the utarray.h macros from uthash?
Could we import the five headers that https://github.com/troydhanson/uthash into contrib and use utarray instead? I know the library is not that efficient since it uses strdup() for all strings that you insert but at least it means not reinventing the wheel.
We already have two copies of uthash.h in the source tree and elftoolchain also includes with utarray.h.

But for now this definitely looks good to me.

This revision is now accepted and ready to land.Oct 10 2019, 9:03 PM

In D21971#480075, @arichardson wrote:

I wish the C standard library had some kind of dynamic array so that this is not reimplement in every program...

I wonder if we could use something like the utarray.h macros from uthash?
Could we import the five headers that https://github.com/troydhanson/uthash into contrib and use utarray instead? I know the library is not that efficient since it uses strdup() for all strings that you insert but at least it means not reinventing the wheel.
We already have two copies of uthash.h in the source tree and elftoolchain also includes with utarray.h.

We certainly could use utarray instead, though I'm not super convinced by its readability.

(Importing uthash.h into contrib is on my todo list since I'd like to merge my changes to use it instead of bdb for hashes in init and a couple other places.)

In D21971#480086, @brooks wrote:

In D21971#480075, @arichardson wrote:

I wish the C standard library had some kind of dynamic array so that this is not reimplement in every program...

I wonder if we could use something like the utarray.h macros from uthash?
Could we import the five headers that https://github.com/troydhanson/uthash into contrib and use utarray instead? I know the library is not that efficient since it uses strdup() for all strings that you insert but at least it means not reinventing the wheel.
We already have two copies of uthash.h in the source tree and elftoolchain also includes with utarray.h.

We certainly could use utarray instead, though I'm not super convinced by its readability.

(Importing uthash.h into contrib is on my todo list since I'd like to merge my changes to use it instead of bdb for hashes in init and a couple other places.)

Sounds good. I agree that utarray is quite ugly (and inefficient) but we could still use it to avoid reimplementing dynamic vectors for non-performance-critical code duplication. I haven't looked if there are any alternatives, I only discovered it because we already use uthash.

jrtc27 added a subscriber: jrtc27.Oct 11 2019, 4:38 PM

jrtc27 added inline comments.

usr.bin/rpcgen/rpc_main.c
924	This is undefined behaviour the first time round as `arglist` is NULL, even though `argcount` is 0. Also, a `realloc`+`memset` should be faster than `calloc`+`memcpy`, whilst also side-stepping this issue (and without having to introduce a branch on `arglist`).

jrtc27 added inline comments.Oct 11 2019, 4:52 PM

usr.bin/rpcgen/rpc_main.c
924	In fact we don't even need the `memset`; `addarg((char *)NULL);` is always called just before the `execvp` so the array will always be NULL-terminated.