Page MenuHomeFreeBSD

Teach ofw_bus_parse_xref_list_alloc to be able to return the length of the parsed list
ClosedPublic

Authored by Sgalabov_gmail.com on Jan 23 2016, 7:05 PM.

Details

Summary

Teach ofw_bus_parse_xref_list_alloc to be able to return the length of the parsed list

Currently, there is no easy way to know in advance how many entries a list parsed by ofw_bus_parse_xref_list_alloc() in sys/dev/ofw/ofw_bus_subr.c has.

The attached patch allows us to pass an idx of -1 to ofw_bus_parse_xref_list_alloc(), in which case it would either return an error (negative) or the number of entries in the parsed list (non-negative).

This would be useful if we don't know in advance how many entries such a list has, but we need to get them (and store them) all - we can call ofw_bus_parse_xref_list_alloc with idx = -1, see how many entries to allocate memory for, and then loop through the entries to get each one of them.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Sgalabov_gmail.com retitled this revision from to Teach ofw_bus_parse_xref_list_alloc to be able to return the length of the parsed list.
Sgalabov_gmail.com updated this object.
Sgalabov_gmail.com edited the test plan for this revision. (Show Details)
Sgalabov_gmail.com added a reviewer: adrian.
Sgalabov_gmail.com set the repository for this revision to rS FreeBSD src repository.

adding some reviwers who have their hands deeper in the ofw code.

andrew edited edge metadata.Jan 24 2016, 12:28 AM

Can you provide more context? It's not obvious what code you're changing.

There are examples of the commands to run to generate the context on the wiki https://wiki.freebsd.org/CodeReview#Create_a_Revision_via_Web_Interface

Sgalabov_gmail.com edited edge metadata.

Updated diff with more context provided.

Please disregard the above patches for now. I'll update and submit a new diff.
The major flaw in the above diffs is that they wrongly assume that the standard error values are negative, which they're not.

Updated diff added. Now we use the ncells parameter to return the number of elements in case idx == -1 is passed as a parameter. This allows us to continue to interpret the return value as simply 0 == success and anything else is a failure, while also being the least intrusive (IMHO) option to allow us to be able to get the length of the list.

Would it be possible for someone to look at this change so we can decide if it's ok to go with it or just forget about it?

adrian added a comment.Feb 5 2016, 4:53 PM

Ideally the API would be changed to return an array that the routine allocates, and the caller can free. But I'm okay with committing this for now until the API gets churned.

Actually, hm, how about we do something a little stranger - let's do this:

  • create a new function, that returns how many entries are there
  • take the function you've created (with idx == -1) and rename it to something internal
  • make the public ofw_bus_parse_xref_list_alloc() call the private version with idx as positive (and asserts on idx < 0); and will return ENOENT as appropriate
  • .. and the new function returns how many entries there are.

How's that sound?

Thanks Adrian, I'll do what you suggest and will submit a new diff.

Diff updated per Adrian's latest suggestion.

Sorry, I attached the wrong patch... Now with the right one.

adrian accepted this revision.Feb 8 2016, 4:22 PM
adrian edited edge metadata.

yup, looks good to me!

This revision is now accepted and ready to land.Feb 8 2016, 4:22 PM
This revision was automatically updated to reflect the committed changes.