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

Make the way exported symbols are specified configurable #1

Closed
wants to merge 6 commits into from

Conversation

nbourdau
Copy link
Contributor

Projects different from Linux Kernel uses different means to specify exported symbol. Often it is done though a function attribute which will indicate if the linker must export or not the symbol in the shared library being built.

This set of patches allows to configure this either from commandline of kernel doc, either from kernel-doc sphinx directive, either from the sphinx configuration.

Copy link
Owner

@return42 return42 left a comment

Choose a reason for hiding this comment

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

Hi @nbourdau, thanks for your excellent PR. I like to merge it, I'am only missing some documentation in ./doc. Can you please add a commit with some documentation & examples- (test-) cases in the ./doc folder / thanks!

and added implicitely to this list of known attributes. The default
list is empty and can be adjusted by the sphinx configuration option
kernel_doc_known_attrs

Copy link
Owner

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

, nargs = "+"
, help = ("provides list of known attributes that has to be"
" hidden from" " function prototypes"))

Copy link
Owner

Choose a reason for hiding this comment

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

Is this " hidden from" " function prototypes" a typo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

* ``EXPORT_SYMBOL(<name>)``
* ``EXPORT_SYMBOL_GPL(<name>)``
* ``EXPORT_SYMBOL_GPL_FUTURE(<name>)``)
pattern specified in ctx_opts
Copy link
Owner

Choose a reason for hiding this comment

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

Another small typo here (ctx_opts).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@return42
Copy link
Owner

@nbourdau in which project do you use linuxdoc?

When matching regular expression looking for exported symbol in macro
used to mark a symbol as exported is matched but also assigned. This
information is not used at all and this prevents some flexibility in the
form of regexp used if we wanted to replace it.

This modifies the regular expression used to find exported symbols to
assign only the exported symbol name.
Pass ParseOption argument to gather_context() function. Now the way an
exported symbol is identified depends on 2 variables in the options
which control how a symbol is specified (by a macro or by a
function/variable attribute) and which identifiers specify an exported
symbol.

The option using macros let the user to specify other macros than
EXPORTED_SYMBOL*() to match an exported symbol, like:
DECLARE_FUNCTION_EXPORTED(exported_function)

The option with attributes change the shape of the regular expression
and allows to recognized exported symbol specified in the function
prototype itself either in definition or in declaration:
API_EXPORTED int exported_function(void* arg) { ...  }

By default, the method is macro and identifiers list is EXPORT_SYMBOL,
EXPORT_SYMBOL_GPL, EXPORT_SYMBOL_GPL_FUTURE which match the previous
behavior used by Linux kernel source code.

This flexibility is necessary to support projects which use kernel-doc
syntax but declare the exported symbols differently.
Add --symbols-exported-method and --symbols-exported-identifiers command
line option to configure the way exported symbols are detected
Add exp-method and exp-ids option to kernel-doc directive for sphinx to
configure the way exported symbol are detected in source code.

The default value can be set globally by kernel_doc_exp_method and
kernel_doc_exp_ids variables.
Allow to specify a list of attributes that, when encountered in the
function prototype, will removed from the displayed function prototype.
When using 'attribute' method to recognise exported symbols, the list of
attribute are implicitely hidden when displaying the function in
_addition_ to the list of known attributes.

This list can be controlled by kernel_doc command line with the
--known-attrs option or in the kernel-doc sphinx directive with the
option known-attrs. The default value in the sphinx directive can be
controlled by the variable kernel_doc_known_attrs.
Use os.getenv and os.path.abspath instead of OS_ENV and ABSPATH from fspath
@nbourdau
Copy link
Contributor Author

nbourdau commented Aug 9, 2018

My bad, github has closed my PR will I was rebasing. Please find the new version with you comments addressed

@nbourdau nbourdau reopened this Aug 9, 2018
@nbourdau
Copy link
Contributor Author

nbourdau commented Aug 9, 2018

@nbourdau in which project do you use linuxdoc?

I start to use it in my company for documenting API and internals of software libraries. In the past we were using kernel-doc syntax but without any doc generator. In the paste I had a look how to develop myself a parser based on kernel-doc script from kernel. When I have discover your project, I find it was a perfect fit for our needs.

I did not have much time in the latest months to finish the adaptation to our needs but now it is fine. You will receive other PR very soon from me and one of my colleagues. I will likely use it in more and more projects (inhouse and opensource)

Also, for your information, in my team, we use gerrit for our developments... For this PR, I have stripped the Change-Id line in the commit message. But do you mind that in future PR, the commit message contains Change-Id line?

@return42
Copy link
Owner

Hi @nbourdau,

applied all except 4295592 thanks! Are there any good reasons to eliminate FSPath? If not, I would like to stay with the FSPath type ... see this slide why.

@return42
Copy link
Owner

Also, for your information, in my team, we use gerrit for our developments... For this PR, I have stripped the Change-Id line in the commit message. But do you mind that in future PR, the commit message contains Change-Id line?

Hm, never used gerrit. Change-ID is new to me. I read the documentation and It seems to me, that Change-IDs outside of the gerrit review-cycle makes no sense. I like to use simple and bare tools in my workflows. Git should be enough, so please strip the Change-ID / thanks!

I also do not use this web based github-PR for merges. I merge locally and before I push, I make some tests. This github-PR GUI was also not able to track my partial merges .. I don't like to fight with such tools ;)

@return42 return42 closed this Aug 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants