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
"Thies C. Arntzen" thies@thieso.net 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
Am 25.03.2005 um 15:28 schrieb Derrell.Lipman@UnwiredUniverse.com:
"Thies C. Arntzen" thies@thieso.net 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.
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
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
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? :) --Janisubject 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
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
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 :)
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.
Wez Furlong wrote:
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 :)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
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