Re: pg_hba options parsing
Bruce Momjian wrote:
OK, a few comments:
This should have spaces:
! port->hba->ldapprefix?port->hba->ldapprefix:"",
Fixed. Though I guess pgindent would've fixed it eventually otherwise.
This is missing 'do' or something:
+ #define MANDATORY_AUTH_ARG(argvar, argname, authname) \ + if (argvar == NULL) {\ + ereport(LOG, \ + (errcode(ERRCODE_CONFIG_FILE_ERROR), \ + errmsg("authentication method '%s' requires argument '%s' to be set", \ + authname, argname), \ + errcontext("line %d of configuration file \"%s\"", \ + line_num, HbaFileName))); \ + goto hba_other_error; \ + } while (0);
Wow.Amazing that it actually compiles and work. I guess it treats the
while(0) as a separate statement completely.
The correct fix is, AFAICS, to remove the while(0).
Seems we are losing the sscanf(), which is good.
Yes, that is one of the goals :-)
If there are no further comments I will go ahead and commit this (with
the above fixes) within a couple of days.
//Magnus
Import Notes
Reply to msg id not found: 200810142214.m9EMEM902310@momjian.usReference msg id not found: 200810142214.m9EMEM902310@momjian.us
Magnus Hagander <magnus@hagander.net> writes:
Bruce Momjian wrote:
This is missing 'do' or something:
+ #define MANDATORY_AUTH_ARG(argvar, argname, authname) \ + if (argvar == NULL) {\ + ereport(LOG, \ + (errcode(ERRCODE_CONFIG_FILE_ERROR), \ + errmsg("authentication method '%s' requires argument '%s' to be set", \ + authname, argname), \ + errcontext("line %d of configuration file \"%s\"", \ + line_num, HbaFileName))); \ + goto hba_other_error; \ + } while (0);
Wow.Amazing that it actually compiles and work. I guess it treats the
while(0) as a separate statement completely.
The correct fix is, AFAICS, to remove the while(0).
Absolutely not! The reason for using do/while in this sort of situation
is to make sure that the "if" can't get matched up to an "else" in code
following the macro. Without do/while this macro will be a loaded
foot-gun.
regards, tom lane
Tom Lane wrote:
Magnus Hagander <magnus@hagander.net> writes:
Bruce Momjian wrote:
This is missing 'do' or something:
+ #define MANDATORY_AUTH_ARG(argvar, argname, authname) \ + if (argvar == NULL) {\ + ereport(LOG, \ + (errcode(ERRCODE_CONFIG_FILE_ERROR), \ + errmsg("authentication method '%s' requires argument '%s' to be set", \ + authname, argname), \ + errcontext("line %d of configuration file \"%s\"", \ + line_num, HbaFileName))); \ + goto hba_other_error; \ + } while (0);Wow.Amazing that it actually compiles and work. I guess it treats the
while(0) as a separate statement completely.The correct fix is, AFAICS, to remove the while(0).
Absolutely not! The reason for using do/while in this sort of situation
is to make sure that the "if" can't get matched up to an "else" in code
following the macro. Without do/while this macro will be a loaded
foot-gun.
Oh, didn't think of that. I just thought of the braces part, which was
"solved" by if. Thanks for clearing that up.
Ok, will add back do/while instead.
//Magnus