seL4 / camkes

Component Architecture test suite and example apps.
https://docs.sel4.systems/CAmkES/
Other
27 stars 37 forks source link

reserved identifier violation #1

Open elfring opened 10 years ago

elfring commented 10 years ago

I would like to point out that identifiers like "_PAYLOAD_" and "_SOCKET_" do not fit to the expected naming convention of the C language standard. Would you like to adjust your selection for unique names?

Smattr commented 10 years ago

Apologies for the late response. It seems none of us were actually on the notify list for this repository.

You are referring to the leading underscore, I take it. Yes, these are reserved names in C. However, in this case they are being used as header guards and not actually present in the post-processed C source. I have no objection to refactoring them to use a different naming scheme, but I also don't see an issue with their current usage.

I'm not sure what you mean by "unique names" here. How does uniqueness factor into this situation?

elfring commented 10 years ago

Include guard macros are also identifiers. How do you think about other interpretations for their relevance in standard compliance?

Would you like to reuse a name pattern that I suggested also for other software?

Smattr commented 10 years ago

seL4/seL4#2 relates to headers inside the kernel. These are clearly part of the implementation, as @lsf37 has said, so underscore-prefixed identifiers are allowed in this context.

The headers referenced in this issue are a bit more of a grey area, as they are not clearly part of the runtime implementation. As I said in my previous response, I have no objection to removing the prefix underscore from these tokens.

I would object to the naming scheme you've proposed as it's unnecessarily complex, in my opinion. It is not an arbitrary, unknown namespace that the header guards are instantiated within. In the case of an application header, the application author knows enough information in advance to avoid colliding with existing symbols. In the case of a library header, prefixing the guard with the name of the library is sufficient to avoid conflicts.

elfring commented 10 years ago

Thanks for your constructive feedback.

I have got a different opinion. I would prefer to reduce the probability for name clashes even more.

Smattr commented 10 years ago

In the case of generated headers, I agree that something more thorough is required, but for these static headers I think what's there is adequate. Even for generated headers though, a guard based on the absolute path of the header is guaranteed to be unique in a localised build. I don't see the need for something more opaque like a UUID. Are you happy for me to close this issue and the other linked ones?

elfring commented 10 years ago

No. - These bug reports can be closed after the affected identifiers will be improved.

Would you like to add the project name to your include guards as a prefix at least?

Smattr commented 10 years ago

This repository itself doesn't need any guard prefixes, as everything in here is an application. That is to say, any author of a header here knows exactly what will and what will not be in scope at the time the header is encountered. I'm open to removing the underscore prefixes on these header guards, but this is quite low priority in relation to some of the other ongoing work.

I'm leaving this issue open for now with the outstanding work being to remove the leading underscores. After that has happened I consider this issue can be closed as resolved.

Smattr commented 8 years ago

Instead of removing the reserved prefixes, these guards should be replaced with #pragma once. This could be done in the generated header as well. If you want to submit a pull request with these changes, I imagine it would be palatable.

elfring commented 8 years ago

Do you care for standard-compliance?