xunzhang / gflags

Automatically exported from code.google.com/p/gflags
BSD 3-Clause "New" or "Revised" License
1 stars 0 forks source link

RegisterFlagValidator warning and Simpler API #64

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Compile the example code in the document about RegisterFlagValidator
static bool ValidatePort(const char* flagname, int32 value) {
   if (value > 0 && value < 32768)   // value is ok
     return true;
   printf("Invalid value for --%s: %d\n", flagname, (int)value);
   return false;
}
DEFINE_int32(port, 0, "What port to listen on");
static const bool port_dummy = RegisterFlagValidator(&FLAGS_port, 
&ValidatePort);

What is the expected output? What do you see instead?
test.cpp:83:19: error: 'port_dummy' defined but not used

What version of the product are you using? On what operating system?
gflags 2.0, linix, gcc 4.5.1

Please provide any additional information below.

I think gflags can provide simpler API, such as:
GFLAGS_register_validator(port, &ValidatePort);

A candidate implementation:
#define GFLAGS_register_validator(name, validator) \
static const bool name##_validator_registered = \
::google::RegisterFlagValidator(&FLAGS_##name, validator) 
__attribute__((unused))

Original issue reported on code.google.com by chen3feng on 14 Jan 2013 at 10:33

GoogleCodeExporter commented 9 years ago
Thanks for the report. I agree that a macro similar to the one you suggested 
makes life easier. However, as the __attribute__ keyword is GCC specific, I 
will look for a more general definition which will work also with other 
compilers.

Original comment by andreas....@gmail.com on 14 Jan 2013 at 2:46

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Maybe introduce another registerer class is a better choice:
class FlagValidatorRegisterer {
 public:
  tempalate <typename T>
  FlagValidatorRegisterer(const bool* flag,
                          bool (*validate_fn)(const char*, T)) {
    success_ = RegisterFlagValidator(flag, validator_fn);
  }
 private:
  bool success_;
};

#define GFLAGS_register_validator(name, validator) \
static FlagValidatorRegisterer 
FLAGS_##name##_validator_registerer(&FLAGS_##name, validator)

Original comment by chen3feng on 16 Jan 2013 at 6:26

GoogleCodeExporter commented 9 years ago
"const bool* flag," should be "const T* flag,"

Original comment by chen3feng on 29 Jan 2013 at 10:22

GoogleCodeExporter commented 9 years ago
FLAGS_##name##_validator_registerer may be potential with other flags variable.
#define GFLAGS_register_validator(name, validator) \
static FlagValidatorRegisterer 
GFLAGS_validator_registerer_##name(&FLAGS_##name, validator)
can resolve this problem, and all validator share the same prefix, make it easy 
for grep.

Original comment by chen3feng on 30 Jan 2013 at 3:00

GoogleCodeExporter commented 9 years ago
Added a DEFINE_validator macro to the API.

Regarding the warning, I have not encountered such when compiling the unit 
tests on a recent Ubuntu and Cygwin installation. This should of course be 
verified yet that the issue is completely resolved before I will close it.

Original comment by andreas....@gmail.com on 20 Mar 2014 at 3:29

GoogleCodeExporter commented 9 years ago

Original comment by andreas....@gmail.com on 20 Mar 2014 at 3:35

GoogleCodeExporter commented 9 years ago
In my case, this warning only issued with compiling errors. thanks for
fixing.

2014-03-20 23:35 GMT+08:00 <gflags@googlecode.com>:

Original comment by chen3feng on 21 Mar 2014 at 2:34

GoogleCodeExporter commented 9 years ago
Only occurs with other compiling errors. if no other errors, nothing warned.

Original comment by chen3feng on 21 Mar 2014 at 2:49