Page MenuHomeFreeBSD

Add string16 API
AbandonedPublic

Authored by eric_metricspace.net on Feb 20 2017, 2:06 PM.
Tags
None
Referenced Files
Unknown Object (File)
Feb 3 2024, 5:26 PM
Unknown Object (File)
Jan 1 2024, 12:39 AM
Unknown Object (File)
Nov 7 2023, 4:59 PM
Unknown Object (File)
Nov 5 2023, 3:31 AM
Unknown Object (File)
Oct 6 2023, 3:46 PM
Unknown Object (File)
Oct 4 2023, 3:29 AM
Unknown Object (File)
Sep 30 2023, 3:28 AM
Unknown Object (File)
Sep 21 2023, 1:03 AM
Subscribers
None

Details

Summary

This patch adds the string16 API used in the larger changeset https://reviews.freebsd.org/D7589. This is a component of that patch.

Test Plan

This should be a behavior-neutral changeset. It can be tested simply by boot-testing boot1.efi and loader.efi.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

eric_metricspace.net retitled this revision from to Add string16 API.
eric_metricspace.net updated this object.
eric_metricspace.net edited the test plan for this revision. (Show Details)
eric_metricspace.net set the repository for this revision to rS FreeBSD src repository - subversion.
imp requested changes to this revision.Feb 20 2017, 4:55 PM
imp edited edge metadata.

All the string functions suffer from the same problem we have with the original string functions you replaced: they don't properly implement character conversions. Please see the efivar stuff I did in -current to see what you need to do there. And you should put the functions I wrote there into libstand. While these functions work well enough for ASCIIish things, they fail for anything more complex. If we're going to rework things, let's do it correctly and reuse what was done correctly for efivar (who, to be fair, stole and reworked the code from Marcel's earlier efi on ia64 work).

This revision now requires changes to proceed.Feb 20 2017, 4:55 PM
In D9687#200246, @imp wrote:

All the string functions suffer from the same problem we have with the original string functions you replaced: they don't properly implement character conversions. Please see the efivar stuff I did in -current to see what you need to do there. And you should put the functions I wrote there into libstand. While these functions work well enough for ASCIIish things, they fail for anything more complex. If we're going to rework things, let's do it correctly and reuse what was done correctly for efivar (who, to be fair, stole and reworked the code from Marcel's earlier efi on ia64 work).

I have additional question... how much the naming is reflected to anything "standard", I mean, sure, the new names do reflect a bit better what is supposed to happen, but could it be we will find ourselves renaming them once again?:)

In D9687#200246, @imp wrote:

All the string functions suffer from the same problem we have with the original string functions you replaced: they don't properly implement character conversions. Please see the efivar stuff I did in -current to see what you need to do there. And you should put the functions I wrote there into libstand. While these functions work well enough for ASCIIish things, they fail for anything more complex. If we're going to rework things, let's do it correctly and reuse what was done correctly for efivar (who, to be fair, stole and reworked the code from Marcel's earlier efi on ia64 work).

Agreed that libstand is probably the right place for all this. The point of me introducing these was to try to standardize things better. If there's already code that does it better, then I should use that instead.

Can you link to the changeset that introduced the efivar stuff (for the sake of other reviewers as well)?

Can you link to the changeset that introduced the efivar stuff (for the sake of other reviewers as well)?

https://svnweb.freebsd.org/base/head/lib/libefivar?revision=307071&view=markup is a good place to start. There were followup commits.

I like the interface in lib/libefi/libefi.c

What about CHAR16 vs u_short? Should this really go into libstand, or would it be more appropriate to put it in lib/libefi and add a dependency to boot/efi/libefi?