Bug #98
closed
reserved identifier violation
Added by Markus Elfring about 14 years ago.
Updated over 13 years ago.
Description
This issue corresponds to my previous "feature request".
I suggest to try the search pattern "\<_(?:(?:_(.*))|([A-Z]+))" on source files. A couple of places will be found where names begin with two underscores or an underscore and an uppercase letter.
Examples:
This does not fit to the expected naming conventions of the C language standard.
See also
Files
|
98.tbz (25.7 KB)
98.tbz |
update suggestion |
Markus Elfring, 06/12/2010 12:10 PM
|
|
Yeah, I think all the headers #ifdef guards can be changed from __FOO_H to CEPH_FOO_H or similar. Patches welcome!
The _WorkQueue can also be renamed easily enough.
No need for the trailing gibberish, I think.. the trailing H and preceding CEPH should be sufficient.
I suggest to append a kind of UUID as a suffix to make include guards really unique.
Do any identifiers with leading underscores belong to a published application programming interface where do you need to keep them for backward compatibility of your source files?
The header ifdef guards should definitely be fixed. I think
#ifndef CEPH_FOO_H
#define CEPH_FOO_H
should be sufficient (i.e. CEPH_ prefix and H suffix). The only conflicts would be someone else using CEPH (their problem) and us using _H (which we don't).
Markus Elfring wrote:
I suggest to append a kind of UUID as a suffix to make include guards really unique.
Do any identifiers with leading underscores belong to a published application programming interface where do you need to keep them for backward compatibility of your source files?
Nope, and even if there are, it's early enough that I'm comfortable breaking them. Feel free to change them.
The qemu guys were worried about this when we submitted the rbd driver. Simply changing __FOO_H to CEPH_FOO_H throughout would be a nice cleanup.
Would you like to integrate any changes from the appended name adjustments into your source code repository?
- Status changed from New to Resolved
Okay, applied these with a few fixes (and without the trailing gibberish in #1). Thanks Markus!
Who is responsible for the tool that generates the affected header files?
Markus Elfring wrote:
Who is responsible for the tool that generates the affected header files?
Oh.. if those are generated headers, it's probably JNI. I wouldn't worry about it in that case (at least not until is actually causes a problem for someone).
Markus Elfring wrote:
Do you mean that a tool for the Java™ Native Interface is affected?
They are generated by some java tool, yeah.
/* DO NOT EDIT THIS FILE - it is machine generated */
#include <jni.h>
/* Header for class org_apache_hadoop_fs_ceph_CephTalker */
#ifndef _Included_org_apache_hadoop_fs_ceph_CephTalker
#define _Included_org_apache_hadoop_fs_ceph_CephTalker
#ifdef __cplusplus
...
Do you use the tool "javah" for these header files?
I don't actually remember how they were created, but according to the Readme file, it's javah, yes.
Those files aren't in the Hadoop repository, though; they're only used in creating the C++ CephFSInterface library which we will manage even if they do accept the userspace client.
Also available in: Atom
PDF