Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions src/dict.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
* POSSIBILITY OF SUCH DAMAGE.
*/

#ifndef __DICT_H
#define __DICT_H
#ifndef DICT_H
#define DICT_H
Copy link
Member

Choose a reason for hiding this comment

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

Why rename this? It's a fairly common pattern in other code. I'm not sure if there is a strong reason to change it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I remember @zuiderkwast requested such similar renaming because the leading double underscore which is a reserved name by the C standard.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mostly meant that when we're anyway renaming things like __REDIS_XYZ, we can drop the leading underscores at the same time.

I don't think we want do it all over the code base. (Sorry if I said that.)

Copy link
Member

Choose a reason for hiding this comment

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

What's the motivation to drop the leading underscores? It's just a little bit less consistent now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it will ever be a problem, unless name macros __ARM or something like that.

If you prefer to keep double underscores for consistency, I can accept that.

Copy link
Contributor

Choose a reason for hiding this comment

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

@lipzhu Please revert back to __DICT_H. If we ever want to change all these reserved names (for some kind of validator in the future) we can fix all files at the same time.

Copy link
Member

Choose a reason for hiding this comment

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

If you prefer to keep double underscores for consistency, I can accept that.

I had read that reserved-names doc multiple times, and never noticed __ clause. I would prefer we keep what we have (since there isn't a conflict), but moving forward use the style that we mentioned here.

Copy link
Contributor

Choose a reason for hiding this comment

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

In case you still didn't find it:

In addition to the names documented in this manual, reserved names include all external identifiers (global functions and variables) that begin with an underscore (‘_’) and all identifiers regardless of use that begin with either two underscores or an underscore followed by a capital letter are reserved names.


#include "mt19937-64.h"
#include <limits.h>
Expand Down Expand Up @@ -210,7 +210,6 @@ void dictSetDoubleVal(dictEntry *de, double val);
int64_t dictIncrSignedIntegerVal(dictEntry *de, int64_t val);
uint64_t dictIncrUnsignedIntegerVal(dictEntry *de, uint64_t val);
double dictIncrDoubleVal(dictEntry *de, double val);
void *dictEntryMetadata(dictEntry *de);
void *dictGetKey(const dictEntry *de);
void *dictGetVal(const dictEntry *de);
int64_t dictGetSignedIntegerVal(const dictEntry *de);
Expand Down Expand Up @@ -253,4 +252,4 @@ void dictFreeStats(dictStats *stats);
int dictTest(int argc, char *argv[], int flags);
#endif

#endif /* __DICT_H */
#endif /* DICT_H */