Bojan Smojver wrote: > As I understand it, PostgreSQL and SQLite3 drivers expect NULL to be the > last value in the the va_list args (for compatibility, I guess other > drivers should expect the same). This value should be placed there by > the caller. The drivers don't take into account any number figured out > in _prepare() as the number of values they should expect in > pvquery/pvselect. The behaviour of va_arg() shouldn't have anything to > do with this.
Hmm ... I'm quite sure what to make of that. Should it be documented, the manner of apr_pstrcat()? What about apr_brigade_vputstrs() too? Moreover, if this is the desired behaviour, then migrating from expecting purely string arguments in apr_dbd_pvquery() and apr_dbd_pvselect() to a sprintf()-style mix of strings and other data types surely isn't going to be easy: how do you distinguish a "no more arguments" NULL from an integer zero? It would seem to me that three options present themselves: 1) Stick with the existing string-only behaviour and document that a trailing NULL must be used for apr_dbd_pvquery() and apr_dbd_pvselect(). Alter the Oracle driver as per my patch in PR 37664, and maybe also alter it to look for trailing NULL arguments in those functions. 2) Stick with the existing string-only behaviour but replace the read-until-NULL logic in certain drivers with loops that read the expected number of arguments, as determined by apr_dbd_prepare(). Alter the Oracle driver as per my patch. 3) Change the read-until-NULL logic as in (2) above, and also change the existing string-only behaviour: i.e., alter the PostgreSQL driver to expect integer arguments when %d is used in apr_dbd_prepare(). Leave the Oracle driver as-is (at least, in relation to this issue ... the rest of my patch still applies. :-) Personally, I think I prefer either (2) or (3). It seems more intuitive to me that if I prepare a SQL statement with sprintf()-style argument specifiers, then I don't also need to supply a trailing NULL in the style of apr_pstrcat(). Another possible wrinkle: Since the PostgreSQL driver currently expects only strings, and has been released in the 1.2 branch, I'm not sure if it's "allowable" to change its behaviour as per (3) for 1.3. It would seem to "kind of" alter the ABI, even though the function definitions don't change (due to the use of va_list). Callers using %d would have to switch from using all strings in their variable argument lists to a mix of strings and integers. I don't know if that qualifies as a binary interface change, but I can see that some might think it would. If it does, then my guess is that only (1) and (2) are allowable as options. Moreover, I'd think that %d would then have to continue to mean "integer passed as string" until version 2.0. Alas! Chris. -- GPG Key ID: 366A375B GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
