Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

combine common api error codes #1151

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bbrcknl
Copy link
Contributor

@bbrcknl bbrcknl commented Dec 17, 2023

The seL4 API has three common error codes

I've replaced the text in the descriptions in the xml files, E.g.

<error name="seL4_IllegalOperation">
    <description>
        The <texttt text='_service'/> is a CPtr to a capability of the wrong type
    </description>
</error>

is now

<error name="seL4_IllegalOperation">
    <description>
        &illegal_operation;
    </description>
</error>

There are a few problems with this approach.

1. Global variable declaration is done at the top of each api xml file. I'm too new to xml and python to place the global variable declaration into a single file (e.g. py code) such that it can be called or added to each api xml file.

2. The text strings that describe error messages are too long. I aimed to include a useful error message + a link to a table with common error codes, e.g.

illegal_operation = "The _service is a CPtr to a capability of the wrong type (see error:illegal_operation in common error codes)"
where error:illegal_operation links to a common error codes table

The long strings are particularly unreadable when there are multiple error codes
E.g.
10.3.3 seL4_IRQControl
10.3.3.1 Get
seL4_IllegalOperation On x86, an IOAPIC is being used. Or, the _service is a CPtr to a capability of the wrong type (see seL4_IllegalOperation in common error codes)

This could be made readable by listing the error codes
E.g.
10.3.3 seL4_IRQControl
10.3.3.1 Get
seL4_IllegalOperation

  • On x86, an IOAPIC is being used.
  • Or, the _service is a CPtr to a capability of the wrong type (see seL4_IllegalOperation in common error codes)

But I'm struggling with the xml and python, can can't figure out how to include lists in the descriptions of errors.

@Indanz @lsf37 Any suggestions/hints?

@lsf37 lsf37 added the docs Manual and other documentation label Feb 6, 2024
@Indanz Indanz mentioned this pull request Mar 12, 2024
@bbrcknl
Copy link
Contributor Author

bbrcknl commented Mar 20, 2024

@indan Zupancic Thanks for the xinclude code
I've tried to use it to include a test fail, but have unfortunately failed.
Can you please take a look at the files I changed in 85c6504 to see what might be going wrong?

@bbrcknl bbrcknl marked this pull request as ready for review April 19, 2024 00:05
Copy link
Contributor

@Indanz Indanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The final result looks okay, but some commits have garbage or other messiness in the that shouldn't be there, it would be nice if you could clean that up, e.g. rebase -i a58480. That would also get rid of the merge commits, which is necessary before we can merge it.

libsel4/arch_include/arm/interfaces/sel4arch.xml Outdated Show resolved Hide resolved
libsel4/arch_include/arm/interfaces/sel4arch.xml Outdated Show resolved Hide resolved
@@ -372,20 +384,21 @@
description="Capability to the scheduling context that the TCB should run on. If the scheduling context is already bound to a notification or TCB that is not this TCB this operation will fail. Similarly, if this TCB is already bound to a scheduling context that is not this scheduling context, this will also fail."/>
<param dir="in" name="fault_ep" type="seL4_CPtr"
description="CPtr to the endpoint which receives IPCs when this thread faults."/>
<error name="seL4_IllegalOperation">
<!-- <error name="seL4_IllegalOperation">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems some stray test change.

- separate common error codes in descriptions
- separate make instructions for latex and markdown
- put error codes into blocks to avoid duplication

Signed-off-by: Birgit Brecknell <b@brck.nl>
Copy link
Contributor

@Indanz Indanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're almost there. You forgot to update a few files (missing git add?), but otherwise looks a lot cleaner now.

libsel4/arch_include/x86/interfaces/sel4arch.xml Outdated Show resolved Hide resolved
libsel4/arch_include/riscv/interfaces/sel4arch.xml Outdated Show resolved Hide resolved
libsel4/include/interfaces/sel4.xml Show resolved Hide resolved
libsel4/sel4_arch_include/x86_64/interfaces/sel4arch.xml Outdated Show resolved Hide resolved
manual/manual.tex Outdated Show resolved Hide resolved
Signed-off-by: Birgit Brecknell <b@brck.nl>
Signed-off-by: Birgit Brecknell <b@brck.nl>
Comment on lines +40 to +42
'docref': self.parse_recurse,
'commonerrorref': self.parse_recurse,
'apiref': self.parse_recurse
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of these recursive ones seem to work, at least not when including the whole common error file, it works for appending error reasons: All generated output is empty, and hence replaced with TODO in the generated PDF. And if this happens, error processing stops and further errors aren't shown at all. :-(

If I remove those elements from the included file I get the expected output.

When it works,the copyright comment also gets inserted into the PDF as readable text, messing things up.

I tried a few different things, but they didn't fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is baffling. I get the pdf attached manual.pdf, which inserts errors as expected, with no TODOs (at least not in the error descriptions).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get manual.pdf. I'm using a Debian 11 Bullseye based distro, I guess there is some version difference in some tool that explains this difference. I have Python 3.9.2. @lsf37 does it work for you? Any ideas?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Manual and other documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants