Code review comment for lp:~stewart/dbd-drizzle/fixup-for-modern-perl

Revision history for this message
Brian Aker (brianaker) wrote :

Go for it.

On Mar 20, 2013, at 7:47 PM, Stewart Smith <email address hidden> wrote:

> Stewart Smith has proposed merging lp:~stewart/dbd-drizzle/fixup-for-modern-perl into lp:dbd-drizzle.
>
> Requested reviews:
> Drizzle Trunk (drizzle-trunk)
>
> For more details, see:
> https://code.launchpad.net/~stewart/dbd-drizzle/fixup-for-modern-perl/+merge/154577
>
> now builds for me with this
> --
> https://code.launchpad.net/~stewart/dbd-drizzle/fixup-for-modern-perl/+merge/154577
> Your team Drizzle Trunk is requested to review the proposed merge of lp:~stewart/dbd-drizzle/fixup-for-modern-perl into lp:dbd-drizzle.
> === modified file 'ChangeLog'
> --- ChangeLog 2012-05-02 09:53:07 +0000
> +++ ChangeLog 2013-03-21 02:46:22 +0000
> @@ -1,3 +1,7 @@
> +2013-03-20 Stewart Smith <email address hidden> (0.305)
> +* Fixed up code to build with more modern perl (sv_yes now PL_sv_yes etc)
> +* Fixed up some #include to point to libdrizzle-1.0 (modern libdrizzle)
> +
> 2011-12-01 Patrick Galbraith <email address hidden> (0.305)
> * Fixed Makefile.PL to conform to changes in libdrizzle directory structure
> * Fixed Makefile.PL to assume 'root' user if not defined
>
> === modified file 'constants.h'
> --- constants.h 2012-05-02 09:53:07 +0000
> +++ constants.h 2013-03-21 02:46:22 +0000
> @@ -2,7 +2,7 @@
> #include "perl.h"
> #include "XSUB.h"
>
> -#include <libdrizzle/drizzle_client.h>
> +#include <libdrizzle-1.0/drizzle_client.h>
>
> static double drizzle_constant(char* name, char* arg) {
> errno = 0;
>
> === modified file 'dbdimp.c'
> --- dbdimp.c 2012-05-02 09:53:07 +0000
> +++ dbdimp.c 2013-03-21 02:46:22 +0000
> @@ -19,6 +19,15 @@
> typedef short WORD;
> #endif
>
> +/* Keep compatibility with older perl */
> +#ifndef PL_sv_yes
> +#define PL_sv_yes sv_yes
> +#endif
> +#ifndef PL_sv_undef
> +#define PL_sv_undef sv_undef
> +#endif
> +
> +
> DBISTATE_DECLARE;
>
> typedef struct sql_type_info_s
> @@ -896,7 +905,7 @@
> {
> SV* sv = DBIc_IMP_DATA(imp_dbh);
>
> - DBIc_set(imp_dbh, DBIcf_AutoCommit, &sv_yes);
> + DBIc_set(imp_dbh, DBIcf_AutoCommit, &PL_sv_yes);
> if (sv && SvROK(sv))
> {
> HV* hv = (HV*) SvRV(sv);
> @@ -1333,7 +1342,7 @@
> D_imp_xxh(drh);
>
> /* The disconnect_all concept is flawed and needs more work */
> - if (!dirty && !SvTRUE(perl_get_sv("DBI::PERL_ENDING",0))) {
> + if (!PL_dirty && !SvTRUE(perl_get_sv("DBI::PERL_ENDING",0))) {
> sv_setiv(DBIc_ERR(imp_drh), (IV)1);
> sv_setpv(DBIc_ERRSTR(imp_drh),
> (char*)"disconnect_all not implemented");
> @@ -1341,7 +1350,7 @@
> DBIc_ERR(imp_drh), DBIc_ERRSTR(imp_drh)); */
> return FALSE;
> }
> - perl_destruct_level = 0;
> + PL_perl_destruct_level = 0;
> return FALSE;
> }
>
> @@ -1576,7 +1585,7 @@
> {
> const char* version = drizzle_con_server_version(imp_dbh->con);
> result= version ?
> - sv_2mortal(newSVpv(version, strlen(version))) : &sv_undef;
> + sv_2mortal(newSVpv(version, strlen(version))) : &PL_sv_undef;
> }
> else if (strEQ(key, "sock"))
> result= sv_2mortal(newSViv((IV) imp_dbh->con));
> @@ -2562,7 +2571,7 @@
> break;
>
> default:
> - sv= &sv_undef;
> + sv= &PL_sv_undef;
> break;
> }
> av_push(av, sv);
> @@ -2576,7 +2585,7 @@
> }
>
> if (av == Nullav)
> - return &sv_undef;
> + return &PL_sv_undef;
>
> return sv_2mortal(newRV_inc((SV*)av));
> }
> @@ -2927,7 +2936,7 @@
> sv= newSVpv((char*) (c), 0); \
> SvREADONLY_on(sv); \
> } else { \
> - sv= &sv_undef; \
> + sv= &PL_sv_undef; \
> } \
> av_push(row, sv);
>
> @@ -3004,7 +3013,7 @@
> IV_PUSH(t->num_prec_radix);
> }
> else
> - av_push(row, &sv_undef);
> + av_push(row, &PL_sv_undef);
>
> IV_PUSH(t->sql_datatype); /* SQL_DATATYPE*/
> IV_PUSH(t->sql_datetime_sub); /* SQL_DATETIME_SUB*/
>
> === modified file 'drizzle.xs'
> --- drizzle.xs 2012-05-02 09:53:07 +0000
> +++ drizzle.xs 2013-03-21 02:46:22 +0000
> @@ -10,7 +10,7 @@
> */
>
> #include <stdlib.h>
> -#include <libdrizzle/drizzle_client.h>
> +#include <libdrizzle-1.0/drizzle_client.h>
> #include "dbdimp.h"
> #include "constants.h"
>
>

« Back to merge proposal