# pdo: patch to make leading colon unneeded for bound variables without breaking BC. - Thread: https://externals.io/message/15597 - Original archive: https://news-web.php.net/php.internals/15597 - Started by Thies C. Arntzen on 2005-03-25T13:20:02+00:00 - Messages: 9 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #15597 — Thies C. Arntzen — 2005-03-25T13:20:02+00:00 URL: https://externals.io/message/15597#15597 subject says it - testcase is also attached. i have updated: sqlite and oci. i have tested: sqlite and postgres (uses bind emulation). i did not look at the firebird code as it seemed "complexish" to me;-) all the other drivers seem to use the emulation (like postgres) so they should work. re, thies --- ## #15602 — Derrell.Lipman@UnwiredUniverse.com — 2005-03-25T14:28:37+00:00 URL: https://externals.io/message/15597#15602 · In reply to: #15597 "Thies C. Arntzen" writes: > diff -u -w -r1.95 pdo_stmt.c > --- ext/pdo/pdo_stmt.c 24 Mar 2005 12:32:06 -0000 1.95 > +++ ext/pdo/pdo_stmt.c 25 Mar 2005 13:13:18 -0000 > @@ -242,6 +242,13 @@ > > hash = is_param ? stmt->bound_params : stmt->bound_columns; > > + if (param->name) { > + if (param->name[0] == ':') { > + param->name = param->name + 1; Sanity check... Without researching thoroughly, this "appears" to warrant further inspection. The field param->name is clearly a pointer since you're able to increment it. It seems likely, then, that param->name was dynamically allocated. If that's not the case, then you can stop reading. If it is, though, then by incrementing param->name, what happens when param->name is freed? I didn't see any place in the patch where it is marked as having been incremented nor anyplace where it is decremented in order to be freed. Cheers, Derrell --- ## #15604 — Thies C.Arntzen — 2005-03-25T15:08:07+00:00 URL: https://externals.io/message/15597#15604 · In reply to: #15602 Am 25.03.2005 um 15:28 schrieb Derrell.Lipman@UnwiredUniverse.com: > "Thies C. Arntzen" writes: > > > […] > > Sanity check... > > Without researching thoroughly, this "appears" to warrant further > inspection. > The field param->name is clearly a pointer since you're able to > increment it. > It seems likely, then, that param->name was dynamically allocated. If > that's > not the case, then you can stop reading. If it is, though, then by > incrementing param->name, what happens when param->name is freed? I > didn't > see any place in the patch where it is marked as having been > incremented nor > anyplace where it is decremented in order to be freed. > > acutally i did a bit of checking... really_register_bound_param is called from two places... and at the end it does: if (param->name) { param->name = estrndup(param->name, param->namelen); so i believe my patch is safe. i agree that it could|should be done nicer... -tc --- ## #15614 — Jani Taskinen — 2005-03-25T23:39:59+00:00 URL: https://externals.io/message/15597#15614 · In reply to: #15597 Excuse me, but what BC? I don't think this stuff has been released yet, so how could you break BC? :) --Jani > subject says it - > testcase is also attached. > > i have updated: sqlite and oci. > i have tested: sqlite and postgres (uses bind emulation). > > i did not look at the firebird code as it seemed "complexish" to me;-) > > all the other drivers seem to use the emulation (like postgres) so they should > work. > > re, thies --- ## #15615 — Wez Furlong — 2005-03-26T00:47:12+00:00 URL: https://externals.io/message/15597#15615 · In reply to: #15614 There are releases on pecl.php.net, and there are (a few) people running these in production. In addition, it's been advertised as working this way for over a year. So, it is important to preserve the current behaviour when this patch is applied (will do so this weekend; need to catch up on a lot of things first). --Wez. > > Excuse me, but what BC? I don't think this stuff has been > released yet, so how could you break BC? :) > > --Jani > > > > > > […] --- ## #15616 — Edin Kadribasic — 2005-03-26T01:13:54+00:00 URL: https://externals.io/message/15597#15616 · In reply to: #15615 > There are releases on pecl.php.net, and there are (a few) people > running these in production. In addition, it's been advertised as > working this way for over a year. > > So, it is important to preserve the current behaviour when this patch > is applied (will do so this weekend; need to catch up on a lot of > things first). Hehe, this is too funny. Last time you dismissed my BC concerns when I said we should remove ":" now by saying something along the lines, we can always make things less strict. If this does not make sense now its because it never did :) Edin --- ## #15620 — Wez Furlong — 2005-03-26T06:07:08+00:00 URL: https://externals.io/message/15597#15620 · In reply to: #15616 > > > > > […] > > Hehe, this is too funny. Last time you dismissed my BC concerns when I > said we should remove ":" now by saying something along the lines, we > can always make things less strict. If this does not make sense now its > because it never did :) I've had code running using ':' since before this topic was first raised. I don't see what's funny about it; the fact is that there is code alive using : prefixes; we can't simply stop supporting it. Thies' patch is simply "making things less strict", and that is not something I had ruled out, just a decision I wanted to defer. --Wez. --- ## #15622 — Thies C.Arntzen — 2005-03-26T07:20:05+00:00 URL: https://externals.io/message/15597#15622 · In reply to: #15615 Am 26.03.2005 um 01:47 schrieb Wez Furlong: > There are releases on pecl.php.net, and there are (a few) people > running these in production. In addition, it's been advertised as > working this way for over a year. > > So, it is important to preserve the current behaviour when this patch > is applied (will do so this weekend; need to catch up on a lot of > things first). actually this patch needs some work (as it's a 20 minute hack on code i've never seen before). i seem to be really unlucky in getting my intention across. by accident i started a thread about: a) bad coding style /and/ b) BC breakage when all i ever wanted was c) to make the colons for bound variables optional... guys, can we please get back to the subject? this list reminds me of driving a _too_ old and broken car: "one hour of driving, 2 hours of fixing" - if it was "one hour of coding, 4 hours of documenting/testing/qa/discussing the right way.." it would be fine by me but right now it seems more like "1 hour of work, 3 hours of comparing dick-sizes with the SmarterGuys(tm)". that is screwed up! thies --- ## #15623 — Lester Caine — 2005-03-26T07:58:38+00:00 URL: https://externals.io/message/15597#15623 · In reply to: #15620 Wez Furlong wrote: > > […] > > I've had code running using ':' since before this topic was first raised. > I don't see what's funny about it; the fact is that there is code > alive using : prefixes; we can't simply stop supporting it. So the way things have been 'standardised' currently are cast in stone, even if maintaining them introduces complex checks across other drivers? > Thies' patch is simply "making things less strict", and that is not > something I had ruled out, just a decision I wanted to defer. I would have thought the reference should be the SQL standard rather than "making things less strict" - Having to ADD the ':' is something I don't remember having to do for some time - in production. Since available time is short, playing with pdo is still being deferred - at least until I can get pdo_firebird to do anything useful ;) -- Lester Caine ----------------------------- L.S.Caine Electronic Services