loader: fix endianness conversion
ClosedPublic
Actions

Authored by royger on Feb 8 2018, 5:22 PM.

Details

Reviewers

emaste
wma
kib
jhb

Commits

rS329060: loader: fix endianness conversion

Summary

r328536 broke symbol loading on amd64 at least (and probably other
arches). r328826 contained the problem to ppc only by adding
pre-processors guards.

Fix this properly by moving the endianness conversion to separate
helper functions, and make the conversion more robust by using sizeof
instead of having to manually code the size of each field.

Finally list the fields in each structure in a macro in order to avoid
code repetition.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

royger created this revision.Feb 8 2018, 5:22 PM

Harbormaster completed remote builds in B14874: Diff 39046.Feb 8 2018, 5:22 PM

kib accepted this revision.Feb 8 2018, 5:39 PM

This revision is now accepted and ready to land.Feb 8 2018, 5:39 PM

Nice!

Existing indentation in this file is a mess, but there are a couple of new cases that don't match either of the two existing styles. I suggest tweaking this change to match the surrounding cases, and then someone (@imp or me, perhaps) should follow up with reformatting the portions that don't conform to style(9).

stand/common/load_elf.c
238 ↗	(On Diff #39046)	indentation - I think this one should be 2 tabs
510 ↗	(On Diff #39046)	indentation
587–588 ↗	(On Diff #39046)	indentation

This works in my QEMU test case and LGTM, modulo indentation nits.

In D14267#299114, @emaste wrote:

Existing indentation in this file is a mess, but there are a couple of new cases that don't match either of the two existing styles. I suggest tweaking this change to match the surrounding cases, and then someone (@imp or me, perhaps) should follow up with reformatting the portions that don't conform to style(9).

Thanks! I've fixed those, but maybe we should think about re-formatting the whole file to match the coding standards. I think @imp has already done that for other loader related files.