From d3bda2c1e8e2aed40bd941587da5e8f5af07ce56 Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Mon, 22 Mar 2021 23:28:19 +0000 Subject: [PATCH 01/17] Initial testing with wrapper around Pg_exec(). --- generic/pgtclCmds.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index bb902f3..c4e832d 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -795,6 +795,10 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) build_param_array(interp, nParams, &objv[index], ¶mValues); } + Tcl_DString ds; + Tcl_Encoding e = Tcl_GetEncoding(interp, "utf-8"); + char *externalString = Tcl_UtfToExternalDString(e, execString, strlen(execString), &ds); + /* we could call PQexecParams when nParams is 0, but PQexecParams * will not accept more than one SQL statement per call, while * PQexec will. by checking and using PQexec when no parameters @@ -802,9 +806,9 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) * use params and might have had multiple statements in a single * request */ if (nParams == 0) { - result = PQexec(conn, execString); + result = PQexec(conn, externalString); } else { - result = PQexecParams(conn, execString, nParams, NULL, paramValues, NULL, NULL, 0); + result = PQexecParams(conn, externalString, nParams, NULL, paramValues, NULL, NULL, 0); ckfree ((void *)paramValues); paramValues = NULL; if(newExecString) { @@ -813,6 +817,9 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) } } + Tcl_FreeEncoding(e); + Tcl_DStringFree(&ds); + connid->sql_count++; /* REPLICATED IN pg_exec_prepared -- NEEDS TO BE FACTORED */ From 110dd3d8fbc66817936c93ee5657d0c26ed66184 Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Tue, 23 Mar 2021 13:35:15 +0000 Subject: [PATCH 02/17] Make utf8 encoding statically set up in Init. --- generic/pgtcl.c | 3 ++- generic/pgtclCmds.c | 13 ++++++++++--- generic/pgtclCmds.h | 2 ++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/generic/pgtcl.c b/generic/pgtcl.c index b807915..41781ee 100644 --- a/generic/pgtcl.c +++ b/generic/pgtcl.c @@ -181,8 +181,9 @@ Pgtcl_Init(Tcl_Interp *interp) if (tclversion >= 8.1) Tcl_PutEnv("PGCLIENTENCODING=UNICODE"); - /* register all pgtcl commands */ + pgtclInitEncoding(interp); + /* register all pgtcl commands */ for (cmdPtr = commands; cmdPtr->name != NULL; cmdPtr++) { Tcl_CreateObjCommand(interp, cmdPtr->name, diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index c4e832d..279f9b0 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -241,6 +241,15 @@ tcl_value(char *value) #define tcl_value(x) x #endif /* TCL_ARRAYS */ +static Tcl_Encoding utf8encoding = NULL; + +/* + * Initialize utf8encoding + */ +void pgtclInitEncoding(Tcl_Interp *interp) { + utf8encoding = Tcl_GetEncoding(interp, "utf-8"); +} + /* * PGgetvalue() * @@ -796,8 +805,7 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) } Tcl_DString ds; - Tcl_Encoding e = Tcl_GetEncoding(interp, "utf-8"); - char *externalString = Tcl_UtfToExternalDString(e, execString, strlen(execString), &ds); + char *externalString = Tcl_UtfToExternalDString(utf8encoding, execString, strlen(execString), &ds); /* we could call PQexecParams when nParams is 0, but PQexecParams * will not accept more than one SQL statement per call, while @@ -817,7 +825,6 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) } } - Tcl_FreeEncoding(e); Tcl_DStringFree(&ds); connid->sql_count++; diff --git a/generic/pgtclCmds.h b/generic/pgtclCmds.h index 2593388..e93d40b 100644 --- a/generic/pgtclCmds.h +++ b/generic/pgtclCmds.h @@ -17,6 +17,8 @@ #include #include "libpq-fe.h" +extern void pgtclInitEncoding(Tcl_Interp *interp); + /* MOVED structure definitions for connection IDs to pctclId.h */ /* **************************/ From 22f3e3c97887cad8fb3a07643c9d6f1a6b4306ec Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Tue, 23 Mar 2021 18:21:38 +0000 Subject: [PATCH 03/17] Mark places to convert (interim), fix tab/space damage. --- generic/pgtclCmds.c | 63 ++++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index 279f9b0..7e4fc99 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -287,6 +287,7 @@ PGgetvalue ( PGresult *result, char *nullString, int tupno, int fieldNumber ) return string; } + // TODO convert from external to utf? /* string is not empty */ return tcl_value (string); } @@ -816,6 +817,7 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) if (nParams == 0) { result = PQexec(conn, externalString); } else { + // TODO convert paramvalues from utf to external result = PQexecParams(conn, externalString, nParams, NULL, paramValues, NULL, NULL, 0); ckfree ((void *)paramValues); paramValues = NULL; @@ -1022,6 +1024,7 @@ Pg_result_foreach(Tcl_Interp *interp, PGresult *result, Tcl_Obj *arrayNameObj, T } char *string = PQgetvalue (result, tupno, column); + // TODO convert from external to utf if (Tcl_SetVar2(interp, arrayName, columnName, string, (TCL_LEAVE_ERR_MSG)) == NULL) { @@ -1303,8 +1306,8 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) return TCL_ERROR; } - /* This will take care of the cleanup */ - Tcl_DeleteCommandFromToken(interp, resultid->cmd_token); + /* This will take care of the cleanup */ + Tcl_DeleteCommandFromToken(interp, resultid->cmd_token); return TCL_OK; } @@ -1328,7 +1331,7 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) } Tcl_SetObjResult(interp, Tcl_NewStringObj( - PQcmdTuples(result), -1)); + PQcmdTuples(result), -1)); return TCL_OK; } @@ -1460,19 +1463,19 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) if (tupno < 0 || tupno >= PQntuples(result)) { - tresult = Tcl_NewStringObj("argument to getTuple cannot exceed ", -1); - Tcl_AppendStringsToObj(tresult, "number of tuples - 1", NULL); - Tcl_SetObjResult(interp, tresult); -/* - Tcl_AppendStringsToObj(Tcl_GetObjResult(interp), - "argument to getTuple cannot exceed ", - "number of tuples - 1", NULL); -*/ + tresult = Tcl_NewStringObj("argument to getTuple cannot exceed ", -1); + Tcl_AppendStringsToObj(tresult, "number of tuples - 1", NULL); + Tcl_SetObjResult(interp, tresult); + /* + Tcl_AppendStringsToObj(Tcl_GetObjResult(interp), + "argument to getTuple cannot exceed ", + "number of tuples - 1", NULL); + */ return TCL_ERROR; } /* set the result object to be an empty list */ - resultObj = Tcl_NewListObj(0, NULL); + resultObj = Tcl_NewListObj(0, NULL); /* build up a return list, Tcl-object-style */ for (i = 0; i < PQnfields(result); i++) @@ -1484,7 +1487,7 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) Tcl_NewStringObj(value, -1)) == TCL_ERROR) return TCL_ERROR; } - Tcl_SetObjResult(interp, resultObj); + Tcl_SetObjResult(interp, resultObj); return TCL_OK; } @@ -1504,8 +1507,8 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) if (tupno < 0 || tupno >= PQntuples(result)) { - tresult = Tcl_NewStringObj("argument to tupleArray cannot exceed number of tuples - 1", -1); - Tcl_SetObjResult(interp, tresult); + tresult = Tcl_NewStringObj("argument to tupleArray cannot exceed number of tuples - 1", -1); + Tcl_SetObjResult(interp, tresult); return TCL_ERROR; } @@ -1535,6 +1538,7 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) char *string; string = PQgetvalue (result, tupno, i); + // TODO convert from external to utf if (*string == '\0') { if (PQgetisnull (result, tupno, i)) { Tcl_UnsetVar2 (interp, arrayName, PQfname(result, i), 0); @@ -1566,7 +1570,7 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) Tcl_ListObjAppendElement(interp, resultObj, Tcl_NewStringObj(PQfname(result, i), -1)); } - Tcl_SetObjResult(interp, resultObj); + Tcl_SetObjResult(interp, resultObj); return TCL_OK; } @@ -1604,7 +1608,7 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) == TCL_ERROR) return TCL_ERROR; } - Tcl_SetObjResult(interp, resultObj); + Tcl_SetObjResult(interp, resultObj); return TCL_OK; } @@ -1761,12 +1765,11 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) if (objc == 3) { if (resultid->nullValueString == NULL || - *resultid->nullValueString == '\0') { - - Tcl_SetObjResult(interp, Tcl_NewStringObj("", 0)); + *resultid->nullValueString == '\0') { + Tcl_SetObjResult(interp, Tcl_NewStringObj("", 0)); } else { - Tcl_SetObjResult(interp, - Tcl_NewStringObj(resultid->nullValueString, -1)); + Tcl_SetObjResult(interp, + Tcl_NewStringObj(resultid->nullValueString, -1)); } return TCL_OK; } @@ -1787,7 +1790,7 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) default: { - Tcl_SetObjResult(interp, Tcl_NewStringObj("Invalid option\n", -1)); + Tcl_SetObjResult(interp, Tcl_NewStringObj("Invalid option\n", -1)); goto Pg_result_errReturn; /* append help info */ } } @@ -1938,6 +1941,7 @@ Pg_execute(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[] * Execute the query */ queryString = Tcl_GetString(objv[i++]); + // TODO convert from utf to external result = PQexec(conn, queryString); connid->sql_count++; @@ -2006,7 +2010,7 @@ Pg_execute(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[] == TCL_ERROR) return TCL_ERROR; - Tcl_SetObjResult(interp, resultObj); + Tcl_SetObjResult(interp, resultObj); PQclear(result); return TCL_ERROR; } @@ -2108,6 +2112,7 @@ execute_put_values(Tcl_Interp *interp, const char *array_varname, { fname = PQfname(result, i); value = PGgetvalue(result, nullValueString, tupno, i); + // TODO convert from external to UTF if (array_varname != NULL) { @@ -3120,8 +3125,10 @@ Pg_select(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) { int status; + // TODO convert from UTF to external // Make the call if (nParams) { + // TODO convert params to external status = PQsendQueryParams(conn, queryString, nParams, NULL, paramValues, NULL, NULL, 0); } else { @@ -3153,8 +3160,10 @@ Pg_select(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) goto cleanup_params_and_return_error; } } else { + // TODO convert from UTF to external // Make the call AND queue up the result. if (nParams) { + // TODO convert params to external result = PQexecParams(conn, queryString, nParams, NULL, paramValues, NULL, NULL, 0); } else { @@ -3303,6 +3312,7 @@ Pg_select(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) } } + // TODO convert from external to UTF if (valueObj == NULL) { valueObj = Tcl_NewStringObj(string, -1); } @@ -3725,6 +3735,7 @@ Pg_sendquery(ClientData cData, Tcl_Interp *interp, int objc, build_param_array(interp, nParams, &objv[index], ¶mValues); } + //TODO convert execString from UTF to external if (nParams == 0) { status = PQsendQuery(conn, execString); } else { @@ -3798,6 +3809,7 @@ Pg_sendquery_prepared(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *C return TCL_ERROR; } + // TODO convert params /* If there are any extra params, allocate paramValues and fill it * with the string representations of all of the extra parameters * substituted on the command line. Otherwise nParams will be 0, @@ -5267,6 +5279,7 @@ Pg_sql(ClientData cData, Tcl_Interp *interp, int objc, binValues = (int *)ckalloc (countbin * sizeof (char *)); for (param = 0; param < count; param++) { + // TODO convert to external paramValues[param] = Tcl_GetString (elemPtrs[param]); if (strcmp(paramValues[param], "NULL") == 0) { @@ -5311,6 +5324,7 @@ Pg_sql(ClientData cData, Tcl_Interp *interp, int objc, Tcl_IncrRefCount(objv[callback]); Tcl_Preserve((ClientData) interp); + // TODO convert to external /* * invoke function based on type * of query @@ -5329,6 +5343,7 @@ Pg_sql(ClientData cData, Tcl_Interp *interp, int objc, } } else { + // TODO convert to external if (prepared) { result = PQexecPrepared(conn, execString, count, paramValues, paramLengths, binValues, binresults); } else if (params) { From f97d1ecf65a2c881416819f42ac3cf0ab56385ac Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Tue, 23 Mar 2021 20:42:59 +0000 Subject: [PATCH 04/17] Simplify management of one-off external/utf sctrings. --- generic/pgtclCmds.c | 55 +++++++++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index 7e4fc99..91a67f3 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -250,6 +250,37 @@ void pgtclInitEncoding(Tcl_Interp *interp) { utf8encoding = Tcl_GetEncoding(interp, "utf-8"); } +// The following two functions "waste" a DStrings storage by not freeing it until it's needed again +// This is a little sloppy but massively simplifies the use since just about every place it's used +// has to handle a possible early error return +// NOTE: only one of these can ever be in flight at any time. + +/* + * Convert one utf string at a time to an external string, hiding the DString management. + */ +char *externalString(char *utfString) +{ + static Tcl_DString tmpds; + static allocated = 0; + if(allocated) Tcl_DStringFree(&tmpds); + allocated = 1; + return Tcl_UtfToExternalDString(utf8encoding, utfString, -1, &tmpds); +} + +/* + * Convert one external string at a time to a utf string, hiding the DString management. + */ +char *utfString(char *externalString) +{ + static Tcl_DString tmpds; + static allocated = 0; + if(allocated) Tcl_DStringFree(&tmpds); + allocated = 1; + return Tcl_ExternalToUtfDString(utf8encoding, externalString, -1, &tmpds); +} + +// TODO something to simplify an array worth of external or utf strings in flight at a time + /* * PGgetvalue() * @@ -287,7 +318,6 @@ PGgetvalue ( PGresult *result, char *nullString, int tupno, int fieldNumber ) return string; } - // TODO convert from external to utf? /* string is not empty */ return tcl_value (string); } @@ -558,11 +588,9 @@ Pg_connect(ClientData cData, Tcl_Interp *interp, int objc, if (async) { conn = PQconnectStart(Tcl_DStringValue(&utfds)); - } else { - conn = PQconnectdb(Tcl_DStringValue(&utfds)); } @@ -572,7 +600,6 @@ Pg_connect(ClientData cData, Tcl_Interp *interp, int objc, return TCL_ERROR; } - Tcl_DStringFree(&utfds); if (PQstatus(conn) != CONNECTION_BAD) @@ -583,7 +610,6 @@ Pg_connect(ClientData cData, Tcl_Interp *interp, int objc, } } - tresult = Tcl_NewStringObj("Connection to database failed\n", -1); if (PQstatus(conn) != CONNECTION_OK) @@ -805,9 +831,6 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) build_param_array(interp, nParams, &objv[index], ¶mValues); } - Tcl_DString ds; - char *externalString = Tcl_UtfToExternalDString(utf8encoding, execString, strlen(execString), &ds); - /* we could call PQexecParams when nParams is 0, but PQexecParams * will not accept more than one SQL statement per call, while * PQexec will. by checking and using PQexec when no parameters @@ -815,10 +838,10 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) * use params and might have had multiple statements in a single * request */ if (nParams == 0) { - result = PQexec(conn, externalString); + result = PQexec(conn, externalString(execString)); } else { // TODO convert paramvalues from utf to external - result = PQexecParams(conn, externalString, nParams, NULL, paramValues, NULL, NULL, 0); + result = PQexecParams(conn, externalString(execString), nParams, NULL, paramValues, NULL, NULL, 0); ckfree ((void *)paramValues); paramValues = NULL; if(newExecString) { @@ -827,8 +850,6 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) } } - Tcl_DStringFree(&ds); - connid->sql_count++; /* REPLICATED IN pg_exec_prepared -- NEEDS TO BE FACTORED */ @@ -1024,9 +1045,8 @@ Pg_result_foreach(Tcl_Interp *interp, PGresult *result, Tcl_Obj *arrayNameObj, T } char *string = PQgetvalue (result, tupno, column); - // TODO convert from external to utf - if (Tcl_SetVar2(interp, arrayName, columnName, string, (TCL_LEAVE_ERR_MSG)) == NULL) + if (Tcl_SetVar2(interp, arrayName, columnName, utfString(string), (TCL_LEAVE_ERR_MSG)) == NULL) { return TCL_ERROR; } @@ -1384,7 +1404,7 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) if (Tcl_ObjSetVar2(interp, arrVarObj, fieldNameObj, Tcl_NewStringObj( - PGgetvalue(result, resultid->nullValueString, tupno, i), + utfString(PGgetvalue(result, resultid->nullValueString, tupno, i)), -1), TCL_LEAVE_ERR_MSG) == NULL) { Tcl_DecrRefCount (fieldNameObj); return TCL_ERROR; @@ -1421,6 +1441,7 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) */ for (tupno = 0; tupno < PQntuples(result); tupno++) { + // TODO convert from external to UTF8 efficiently const char *field0 = PGgetvalue(result, resultid->nullValueString, tupno, 0); for (i = 1; i < PQnfields(result); i++) @@ -1482,8 +1503,8 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) { char *value; - value = PGgetvalue(result, resultid->nullValueString, tupno, i); - if (Tcl_ListObjAppendElement(interp, resultObj, + value = utfString(PGgetvalue(result, resultid->nullValueString, tupno, i)); + if (Tcl_ListObjAppendElement(interp, resultObj, Tcl_NewStringObj(value, -1)) == TCL_ERROR) return TCL_ERROR; } From f2264477cc2e83217c015b22a0f583d1ceac1475 Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Tue, 23 Mar 2021 20:56:59 +0000 Subject: [PATCH 05/17] Add required consts. --- generic/pgtclCmds.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index 91a67f3..b900ae1 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -258,10 +258,10 @@ void pgtclInitEncoding(Tcl_Interp *interp) { /* * Convert one utf string at a time to an external string, hiding the DString management. */ -char *externalString(char *utfString) +char *externalString(const char *utfString) { static Tcl_DString tmpds; - static allocated = 0; + static int allocated = 0; if(allocated) Tcl_DStringFree(&tmpds); allocated = 1; return Tcl_UtfToExternalDString(utf8encoding, utfString, -1, &tmpds); @@ -270,10 +270,10 @@ char *externalString(char *utfString) /* * Convert one external string at a time to a utf string, hiding the DString management. */ -char *utfString(char *externalString) +char *utfString(const char *externalString) { static Tcl_DString tmpds; - static allocated = 0; + static int allocated = 0; if(allocated) Tcl_DStringFree(&tmpds); allocated = 1; return Tcl_ExternalToUtfDString(utf8encoding, externalString, -1, &tmpds); From f932368bbaec7a41a8bd2f1997a0ee188dd9e6aa Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Wed, 24 Mar 2021 13:53:47 +0000 Subject: [PATCH 06/17] Snapshot-converted up to pg_execute --- generic/pgtclCmds.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index b900ae1..5e121aa 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -1416,7 +1416,6 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) case OPT_ASSIGNBYIDX: { - if ((objc != 4) && (objc != 5)) { Tcl_WrongNumArgs(interp, 3, objv, "arrayName ?append_string?"); @@ -1441,8 +1440,10 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) */ for (tupno = 0; tupno < PQntuples(result); tupno++) { - // TODO convert from external to UTF8 efficiently - const char *field0 = PGgetvalue(result, resultid->nullValueString, tupno, 0); + const char *rawField0 = PGgetvalue(result, resultid->nullValueString, tupno, 0); + char *field0 = ckalloc(strlen(rawField0)*4+1); + + strcpy(field0, utfString(rawField0)); for (i = 1; i < PQnfields(result); i++) { @@ -1457,14 +1458,16 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) if (appendstrObj != NULL) Tcl_AppendObjToObj(fieldNameObj, appendstrObj); + char *val = utfString(PGgetvalue(result, resultid->nullValueString, tupno, i)); if (Tcl_ObjSetVar2(interp, arrVarObj, fieldNameObj, - Tcl_NewStringObj( PGgetvalue(result, resultid->nullValueString, tupno, i), -1), TCL_LEAVE_ERR_MSG) == NULL) + Tcl_NewStringObj( val, -1), TCL_LEAVE_ERR_MSG) == NULL) { - Tcl_DecrRefCount(fieldNameObj); + ckfree(field0); return TCL_ERROR; } } + ckfree(field0); } return TCL_OK; } @@ -1505,7 +1508,7 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) value = utfString(PGgetvalue(result, resultid->nullValueString, tupno, i)); if (Tcl_ListObjAppendElement(interp, resultObj, - Tcl_NewStringObj(value, -1)) == TCL_ERROR) + Tcl_NewStringObj(utfString(value), -1)) == TCL_ERROR) return TCL_ERROR; } Tcl_SetObjResult(interp, resultObj); @@ -1544,8 +1547,10 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) for (i = 0; i < PQnfields(result); i++) { if (Tcl_SetVar2(interp, arrayName, PQfname(result, i), - PGgetvalue(result, resultid->nullValueString, - tupno, i), TCL_LEAVE_ERR_MSG) == NULL) + utfString( + PGgetvalue(result, resultid->nullValueString, + tupno, i) + ), TCL_LEAVE_ERR_MSG) == NULL) return TCL_ERROR; } } else @@ -1559,13 +1564,13 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) char *string; string = PQgetvalue (result, tupno, i); - // TODO convert from external to utf if (*string == '\0') { if (PQgetisnull (result, tupno, i)) { Tcl_UnsetVar2 (interp, arrayName, PQfname(result, i), 0); continue; } } + string = utfString(string); if (Tcl_SetVar2(interp, arrayName, PQfname(result, i), string, @@ -1656,7 +1661,7 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) { fieldObj = Tcl_NewObj(); - Tcl_SetStringObj(fieldObj, PGgetvalue(result, resultid->nullValueString, tupno, i), -1); + Tcl_SetStringObj(fieldObj, utfString(PGgetvalue(result, resultid->nullValueString, tupno, i)), -1); if (Tcl_ListObjAppendElement(interp, listObj, fieldObj) != TCL_OK) { Tcl_DecrRefCount(listObj); @@ -1698,7 +1703,7 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) fieldObj = Tcl_NewObj(); - Tcl_SetStringObj(fieldObj, PGgetvalue(result, resultid->nullValueString, tupno, i), -1); + Tcl_SetStringObj(fieldObj, utfString(PGgetvalue(result, resultid->nullValueString, tupno, i)), -1); if (Tcl_ListObjAppendElement(interp, subListObj, fieldObj) != TCL_OK) { @@ -1749,7 +1754,7 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) fieldNameObj = Tcl_NewObj(); Tcl_SetStringObj(fieldNameObj, PQfname(result, i), -1); - Tcl_SetStringObj(fieldObj, PGgetvalue(result, resultid->nullValueString, tupno, i), -1); + Tcl_SetStringObj(fieldObj, utfString(PGgetvalue(result, resultid->nullValueString, tupno, i)), -1); if (Tcl_DictObjPut(interp, subListObj, fieldNameObj, fieldObj) != TCL_OK) { @@ -1843,6 +1848,7 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) return TCL_ERROR; } +//HERE// /********************************** * pg_execute From ce2f3d7ba687e838e9c59da9168d7b957ac668de Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Wed, 24 Mar 2021 14:36:46 +0000 Subject: [PATCH 07/17] Converted pg_execute and pg_select except for parameters. --- generic/pgtclCmds.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index 5e121aa..8ae6cea 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -1848,8 +1848,6 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) return TCL_ERROR; } -//HERE// - /********************************** * pg_execute send a query string to the backend connection and process the result @@ -1954,20 +1952,20 @@ Pg_execute(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[] Tcl_SetObjResult(interp, Tcl_NewStringObj("Attempt to query while COPY in progress", -1)); - return TCL_ERROR; + return TCL_ERROR; } if (connid->callbackPtr || connid->callbackInterp) { Tcl_SetResult(interp, "Attempt to query while waiting for callback", TCL_STATIC); return TCL_ERROR; - } + } /* * Execute the query */ - queryString = Tcl_GetString(objv[i++]); + queryString = externalString(Tcl_GetString(objv[i++])); // TODO convert from utf to external result = PQexec(conn, queryString); connid->sql_count++; @@ -2138,8 +2136,7 @@ execute_put_values(Tcl_Interp *interp, const char *array_varname, for (i = 0; i < n; i++) { fname = PQfname(result, i); - value = PGgetvalue(result, nullValueString, tupno, i); - // TODO convert from external to UTF + value = utfString(PGgetvalue(result, nullValueString, tupno, i)); if (array_varname != NULL) { @@ -3147,15 +3144,18 @@ Pg_select(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) } } + if(nParams) { + // TODO convert parameters to external + } + queryString = externalString(queryString); + connid->sql_count++; if (rowByRow) { int status; - // TODO convert from UTF to external // Make the call if (nParams) { - // TODO convert params to external status = PQsendQueryParams(conn, queryString, nParams, NULL, paramValues, NULL, NULL, 0); } else { @@ -3187,10 +3187,8 @@ Pg_select(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) goto cleanup_params_and_return_error; } } else { - // TODO convert from UTF to external // Make the call AND queue up the result. if (nParams) { - // TODO convert params to external result = PQexecParams(conn, queryString, nParams, NULL, paramValues, NULL, NULL, 0); } else { @@ -3339,9 +3337,8 @@ Pg_select(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) } } - // TODO convert from external to UTF if (valueObj == NULL) { - valueObj = Tcl_NewStringObj(string, -1); + valueObj = Tcl_NewStringObj(utfString(string), -1); } if (Tcl_ObjSetVar2(interp, varNameObj, columnNameObjs[column], @@ -3416,6 +3413,7 @@ Pg_select(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) return retval; } +//HERE// /* * Test whether any callbacks are registered on this connection for * the given relation name. NB: supplied name must be case-folded already. From aab8062d466d642a72f49ec40269ad4a393fc241 Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Wed, 24 Mar 2021 19:05:03 +0000 Subject: [PATCH 08/17] Finished wrapping all query strings. --- generic/pgtclCmds.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index 8ae6cea..7b08c52 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -702,6 +702,7 @@ void build_param_array(Tcl_Interp *interp, int nParams, Tcl_Obj *CONST objv[], c paramValues = (const char **)ckalloc (nParams * sizeof (char *)); + // TODO convert to external for (param = 0; param < nParams; param++) { paramValues[param] = Tcl_GetString(objv[param]); if (strcmp(paramValues[param], "NULL") == 0) @@ -3413,7 +3414,6 @@ Pg_select(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) return retval; } -//HERE// /* * Test whether any callbacks are registered on this connection for * the given relation name. NB: supplied name must be case-folded already. @@ -3760,11 +3760,10 @@ Pg_sendquery(ClientData cData, Tcl_Interp *interp, int objc, build_param_array(interp, nParams, &objv[index], ¶mValues); } - //TODO convert execString from UTF to external if (nParams == 0) { - status = PQsendQuery(conn, execString); + status = PQsendQuery(conn, externalString(execString)); } else { - status = PQsendQueryParams(conn, execString, nParams, NULL, paramValues, NULL, NULL, 1); + status = PQsendQueryParams(conn, externalString(execString), nParams, NULL, paramValues, NULL, NULL, 1); if(newExecString) ckfree(newExecString); ckfree ((void *)paramValues); } @@ -3833,6 +3832,7 @@ Pg_sendquery_prepared(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *C Tcl_SetResult(interp, "Attempt to query while COPY in progress", TCL_STATIC); return TCL_ERROR; } +//HERE// // TODO convert params /* If there are any extra params, allocate paramValues and fill it @@ -5349,7 +5349,6 @@ Pg_sql(ClientData cData, Tcl_Interp *interp, int objc, Tcl_IncrRefCount(objv[callback]); Tcl_Preserve((ClientData) interp); - // TODO convert to external /* * invoke function based on type * of query @@ -5357,24 +5356,23 @@ Pg_sql(ClientData cData, Tcl_Interp *interp, int objc, if (prepared) { iResult = PQsendQueryPrepared(conn, execString, count, paramValues, paramLengths, binValues, binresults); } else if (params) { - iResult = PQsendQueryParams(conn, execString, count, NULL, paramValues, paramLengths, binValues, binresults); + iResult = PQsendQueryParams(conn, externalString(execString), count, NULL, paramValues, paramLengths, binValues, binresults); } else { - iResult = PQsendQuery(conn, execString); + iResult = PQsendQuery(conn, externalString(execString)); /* ckfree ((void *)paramValues); */ } } else { - // TODO convert to external if (prepared) { result = PQexecPrepared(conn, execString, count, paramValues, paramLengths, binValues, binresults); } else if (params) { - result = PQexecParams(conn, execString, count, NULL, paramValues, paramLengths, binValues, binresults); + result = PQexecParams(conn, externalString(execString), count, NULL, paramValues, paramLengths, binValues, binresults); } else { - result = PQexec(conn, execString); + result = PQexec(conn, externalString(execString)); ckfree ((void *)paramValues); } } /* end if callback */ From babda1bcf7b8b04fc704721a2bfb0a64f0ea7bf9 Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Wed, 24 Mar 2021 19:32:33 +0000 Subject: [PATCH 09/17] Process ids are bigger than they used to be. --- tests/pgtcl.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/pgtcl.test b/tests/pgtcl.test index 32ebeef..6926ff8 100644 --- a/tests/pgtcl.test +++ b/tests/pgtcl.test @@ -1101,7 +1101,7 @@ test pgtcl-9.4 {dbinfo backend pid} -body { lappend res [string equal $val $val2] ::pg::disconnect $conn - lappend res [regexp {^[0-9]{2,6}$} $val] + lappend res [regexp {^[0-9]{2,7}$} $val] } -result [list 1 1] From 80b8818794d7f32a50b1241304b7e976ed4a14eb Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Wed, 24 Mar 2021 19:34:14 +0000 Subject: [PATCH 10/17] Don't call utfString on a utfString. --- generic/pgtclCmds.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index 7b08c52..c713314 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -1508,8 +1508,9 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) char *value; value = utfString(PGgetvalue(result, resultid->nullValueString, tupno, i)); + if (Tcl_ListObjAppendElement(interp, resultObj, - Tcl_NewStringObj(utfString(value), -1)) == TCL_ERROR) + Tcl_NewStringObj(value, -1)) == TCL_ERROR) return TCL_ERROR; } Tcl_SetObjResult(interp, resultObj); From 4b932b3ae86a40bfbf8b0988e43a2da2af5e3e28 Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Thu, 25 Mar 2021 17:34:13 +0000 Subject: [PATCH 11/17] Convert build_param_array to do UTF to utf-8 conversions. --- generic/pgtclCmds.c | 106 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 92 insertions(+), 14 deletions(-) diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index c713314..0784435 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -36,7 +36,7 @@ static int expand_parameters(Tcl_Interp *interp, const char *queryString, int nParams, char *paramArrayName, char **newQueryStringPtr, const char ***paramValuesPtr); -static void build_param_array(Tcl_Interp *interp, int nParams, Tcl_Obj *CONST objv[], const char ***paramValuesPtr); +static int build_param_array(Tcl_Interp *interp, int nParams, Tcl_Obj *CONST objv[], const char ***paramValuesPtr, const char **bufferPtr); static void report_connection_error(Tcl_Interp *interp, PGconn *conn); @@ -611,7 +611,7 @@ Pg_connect(ClientData cData, Tcl_Interp *interp, int objc, } - tresult = Tcl_NewStringObj("Connection to database failed\n", -1); + tresult = Tcl_NewStringObj("Connection to database failed\n", -1); if (PQstatus(conn) != CONNECTION_OK) { Tcl_AppendStringsToObj(tresult, PQerrorMessage(conn), NULL); @@ -690,27 +690,81 @@ Pg_disconnect(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST obj * with the string representations of all of the extra parameters * substituted on the command line. Otherwise nParams will be 0, * and PQexecParams will work just like PQexec (no $-substitutions). - * The magic string NULL is replaced by a null value! + * The magic string NULL is replaced by a null value! // TODO - make this use null value string */ -void build_param_array(Tcl_Interp *interp, int nParams, Tcl_Obj *CONST objv[], const char ***paramValuesPtr) +int build_param_array(Tcl_Interp *interp, int nParams, Tcl_Obj *CONST objv[], const char ***paramValuesPtr, const char **bufferPtr) { const char **paramValues = NULL; int param; + // For converting Tcl's not actually utf-8 into real utf-8 + int *paramLengths = NULL; + int charsWritten; + char *nextDestByte, *paramsBuffer; + int remaining; + int lengthRequired = 0; + if(nParams == 0) - return; + return TCL_OK; paramValues = (const char **)ckalloc (nParams * sizeof (char *)); + paramLengths = (int *)ckalloc(nParams * sizeof(int)); - // TODO convert to external for (param = 0; param < nParams; param++) { - paramValues[param] = Tcl_GetString(objv[param]); + int newLength = 0; + paramValues[param] = Tcl_GetStringFromObj(objv[param], &newLength); if (strcmp(paramValues[param], "NULL") == 0) { paramValues[param] = NULL; + paramLengths[param] = 0; } + else + { + // per dgp Tcl "UTF8" always takes up more room than real utf-8. + lengthRequired += newLength + 1; + paramLengths[param] = newLength; + } + } + + lengthRequired += 4; //(Tcl_UtfToExternal assumes it will need 4 bytes for the last character) + + nextDestByte = paramsBuffer = (char *)ckalloc(lengthRequired); + remaining = lengthRequired; + + for(param = 0; param < nParams; param++) { + int errcode; + if(!paramLengths[param]) { + continue; + } + // the arguments to Tcl_UtfToExternal are hellish + if( TCL_OK != (errcode = Tcl_UtfToExternal(interp, utf8encoding, paramValues[param], paramLengths[param], 0, NULL, nextDestByte, remaining, NULL, &charsWritten, NULL))) { + Tcl_Obj *tresult; + char errmsg[256]; + + sprintf(errmsg, "Errcode %d attempting to convert param %d: ", errcode, param); + tresult = Tcl_NewStringObj(errmsg, -1); + Tcl_AppendStringsToObj(tresult, paramValues[param], NULL); + if(errcode == TCL_CONVERT_NOSPACE) { // CAN'T HAPPEN, check anyway + sprintf(errmsg, " (%d bytes needed, %d bytes available)", paramLengths[param], remaining); + Tcl_AppendStringsToObj(tresult, errmsg, NULL); + } + Tcl_SetObjResult(interp, tresult); + + ckfree(paramValues); + ckfree(paramLengths); + ckfree(paramsBuffer); + + return errcode; + } + paramValues[param] = nextDestByte; + nextDestByte += charsWritten; + *nextDestByte++ = '\0'; + remaining -= charsWritten + 1; } + *bufferPtr = paramsBuffer; *paramValuesPtr = paramValues; + + return TCL_OK; } /********************************** @@ -735,6 +789,7 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) char *newExecString = NULL; const char **paramValues = NULL; char *paramArrayName = NULL; + const char *paramsBuffer = NULL; int nParams; int index; int useVariables = 0; @@ -828,8 +883,10 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) execString = newExecString; } } else if (nParams) { - // After this point we must free paramValues before exiting - build_param_array(interp, nParams, &objv[index], ¶mValues); + if (build_param_array(interp, nParams, &objv[index], ¶mValues, ¶msBuffer) != TCL_OK) { + return TCL_ERROR; + } + // After this point we must free paramValues and paramsBufferbefore exiting } /* we could call PQexecParams when nParams is 0, but PQexecParams @@ -851,6 +908,11 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) } } + if(paramsBuffer) { + ckfree ((void *)paramsBuffer); + paramsBuffer = NULL; + } + connid->sql_count++; /* REPLICATED IN pg_exec_prepared -- NEEDS TO BE FACTORED */ @@ -932,6 +994,7 @@ Pg_exec_prepared(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST const char *connString; const char *statementNameString; const char **paramValues = NULL; + const char *paramsBuffer = NULL; int nParams; @@ -965,8 +1028,10 @@ Pg_exec_prepared(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST nParams = objc - 3; if (nParams > 0) { - // After this point we must free paramValues before exiting - build_param_array(interp, nParams, &objv[3], ¶mValues); + if (build_param_array(interp, nParams, &objv[3], ¶mValues, ¶msBuffer) != TCL_OK) { + return TCL_ERROR; + } + // After this point we must free paramValues and paramsBufferbefore exiting } statementNameString = Tcl_GetString(objv[2]); @@ -977,6 +1042,11 @@ Pg_exec_prepared(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST ckfree ((void *)paramValues); } + if(paramsBuffer) { + ckfree ((void *)paramsBuffer); + paramsBuffer = NULL; + } + connid->sql_count++; /* REPLICATED IN pg_exec -- NEEDS TO BE FACTORED */ @@ -3024,6 +3094,7 @@ Pg_select(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) const char *queryString = NULL; char *varNameString = NULL; char *paramArrayName = NULL; + const char *paramsBuffer = NULL; Tcl_Obj *varNameObj = NULL; Tcl_Obj *procStringObj = NULL; Tcl_Obj *columnListObj = NULL; @@ -3120,7 +3191,9 @@ Pg_select(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) if (Tcl_ListObjGetElements(interp, paramListObj, &nParams, &listObjv) == TCL_ERROR) { return TCL_ERROR; } - build_param_array(interp, nParams, listObjv, ¶mValues); + if (build_param_array(interp, nParams, listObjv, ¶mValues, ¶msBuffer) != TCL_OK) { + return TCL_ERROR; + } } if(paramArrayName) { @@ -3141,6 +3214,7 @@ Pg_select(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) if (conn == NULL) { cleanup_params_and_return_error: { if(paramValues) ckfree((void *)paramValues); + if(paramsBuffer) ckfree((void *)paramsBuffer); if(newQueryString) ckfree((void *)newQueryString); return TCL_ERROR; } @@ -3664,6 +3738,7 @@ Pg_sendquery(ClientData cData, Tcl_Interp *interp, int objc, char *newExecString = NULL; const char **paramValues = NULL; char *paramArrayName = NULL; + const char *paramsBuffer = NULL; int nParams; int index; int useVariables = 0; @@ -3757,8 +3832,10 @@ Pg_sendquery(ClientData cData, Tcl_Interp *interp, int objc, execString = newExecString; } } else if (nParams) { - // After this point we must free paramValues before exiting - build_param_array(interp, nParams, &objv[index], ¶mValues); + // After this point we must free paramValues and paramsBuffer before exiting + if (build_param_array(interp, nParams, &objv[index], ¶mValues, ¶msBuffer) != TCL_OK) { + return TCL_ERROR; + } } if (nParams == 0) { @@ -3768,6 +3845,7 @@ Pg_sendquery(ClientData cData, Tcl_Interp *interp, int objc, if(newExecString) ckfree(newExecString); ckfree ((void *)paramValues); } + if(paramsBuffer) ckfree((void *)paramsBuffer); connid->sql_count++; /* Transfer any notify events from libpq to Tcl event queue. */ From 4dae59e7b22f9043df6be00e53d57f73fb4fd5dd Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Thu, 25 Mar 2021 18:35:10 +0000 Subject: [PATCH 12/17] Extract convesrion code into new array_to_utf function, add call to array_to_utf for expand_parameters. --- generic/pgtclCmds.c | 109 +++++++++++++++++++++++++++----------------- 1 file changed, 66 insertions(+), 43 deletions(-) diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index 0784435..800e679 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -34,7 +34,8 @@ static int count_parameters(Tcl_Interp *interp, const char *queryString, static int expand_parameters(Tcl_Interp *interp, const char *queryString, int nParams, char *paramArrayName, - char **newQueryStringPtr, const char ***paramValuesPtr); + char **newQueryStringPtr, const char ***paramValuesPtr, + const char **bufferPtr); static int build_param_array(Tcl_Interp *interp, int nParams, Tcl_Obj *CONST objv[], const char ***paramValuesPtr, const char **bufferPtr); @@ -685,45 +686,20 @@ Pg_disconnect(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST obj return TCL_OK; } -/* build_param_array - helper for pg_exec and pg_sendquery */ -/* If there are any extra params, allocate paramValues and fill it - * with the string representations of all of the extra parameters - * substituted on the command line. Otherwise nParams will be 0, - * and PQexecParams will work just like PQexec (no $-substitutions). - * The magic string NULL is replaced by a null value! // TODO - make this use null value string - */ -int build_param_array(Tcl_Interp *interp, int nParams, Tcl_Obj *CONST objv[], const char ***paramValuesPtr, const char **bufferPtr) +/* helper for build_param_array and other related functions. +** convert nParams strings in paramValues, lengths in paramLengths, +** return allocated buffer containing new strings in bufferPtr +*/ +int array_to_utf8(Tcl_Interp *interp, const char **paramValues, int *paramLengths, int nParams, const char **bufferPtr) { - const char **paramValues = NULL; - int param; - - // For converting Tcl's not actually utf-8 into real utf-8 - int *paramLengths = NULL; + int param; int charsWritten; char *nextDestByte, *paramsBuffer; int remaining; int lengthRequired = 0; - if(nParams == 0) - return TCL_OK; - - paramValues = (const char **)ckalloc (nParams * sizeof (char *)); - paramLengths = (int *)ckalloc(nParams * sizeof(int)); - for (param = 0; param < nParams; param++) { - int newLength = 0; - paramValues[param] = Tcl_GetStringFromObj(objv[param], &newLength); - if (strcmp(paramValues[param], "NULL") == 0) - { - paramValues[param] = NULL; - paramLengths[param] = 0; - } - else - { - // per dgp Tcl "UTF8" always takes up more room than real utf-8. - lengthRequired += newLength + 1; - paramLengths[param] = newLength; - } + lengthRequired += paramLengths[param] + 1; } lengthRequired += 4; //(Tcl_UtfToExternal assumes it will need 4 bytes for the last character) @@ -745,15 +721,12 @@ int build_param_array(Tcl_Interp *interp, int nParams, Tcl_Obj *CONST objv[], co tresult = Tcl_NewStringObj(errmsg, -1); Tcl_AppendStringsToObj(tresult, paramValues[param], NULL); if(errcode == TCL_CONVERT_NOSPACE) { // CAN'T HAPPEN, check anyway - sprintf(errmsg, " (%d bytes needed, %d bytes available)", paramLengths[param], remaining); - Tcl_AppendStringsToObj(tresult, errmsg, NULL); + sprintf(errmsg, " (%d bytes needed, %d bytes available)", paramLengths[param], remaining); + Tcl_AppendStringsToObj(tresult, errmsg, NULL); } Tcl_SetObjResult(interp, tresult); - ckfree(paramValues); - ckfree(paramLengths); ckfree(paramsBuffer); - return errcode; } paramValues[param] = nextDestByte; @@ -761,7 +734,50 @@ int build_param_array(Tcl_Interp *interp, int nParams, Tcl_Obj *CONST objv[], co *nextDestByte++ = '\0'; remaining -= charsWritten + 1; } + *bufferPtr = paramsBuffer; + return TCL_OK; +} + +/* build_param_array - helper for pg_exec and pg_sendquery */ +/* If there are any extra params, allocate paramValues and fill it + * with the string representations of all of the extra parameters + * substituted on the command line. Otherwise nParams will be 0, + * and PQexecParams will work just like PQexec (no $-substitutions). + * The magic string NULL is replaced by a null value! // TODO - make this use null value string + */ +int build_param_array(Tcl_Interp *interp, int nParams, Tcl_Obj *CONST objv[], const char ***paramValuesPtr, const char **bufferPtr) +{ + const char **paramValues = NULL; + int *paramLengths = NULL; + int param; + + if(nParams == 0) + return TCL_OK; + + paramValues = (const char **)ckalloc (nParams * sizeof (char *)); + paramLengths = (int *)ckalloc(nParams * sizeof(int)); + + for (param = 0; param < nParams; param++) { + int newLength = 0; + paramValues[param] = Tcl_GetStringFromObj(objv[param], &newLength); + if (strcmp(paramValues[param], "NULL") == 0) + { + paramValues[param] = NULL; + paramLengths[param] = 0; + } + else + { + paramLengths[param] = newLength; + } + } + + if (array_to_utf8(interp, paramValues, paramLengths, nParams, bufferPtr) != TCL_OK) { + ckfree(paramValues); + ckfree(paramLengths); + return TCL_ERROR; + } + *paramValuesPtr = paramValues; return TCL_OK; @@ -877,7 +893,7 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) } if(nParams) { // After this point we must free newExecString and paramValues before exiting - if (expand_parameters(interp, execString, nParams, paramArrayName, &newExecString, ¶mValues) == TCL_ERROR) { + if (expand_parameters(interp, execString, nParams, paramArrayName, &newExecString, ¶mValues, ¶msBuffer) == TCL_ERROR) { return TCL_ERROR; } execString = newExecString; @@ -2934,11 +2950,12 @@ static int count_parameters(Tcl_Interp *interp, const char *queryString, int *nP If not, does not modify the arguments. */ static int expand_parameters(Tcl_Interp *interp, const char *queryString, int nParams, char *paramArrayName, - char **newQueryStringPtr, const char ***paramValuesPtr) + char **newQueryStringPtr, const char ***paramValuesPtr, const char **bufferPtr) { // Allocating space for parameter IDs up to 100,000 (5 characters) char *newQueryString = (char *)ckalloc(strlen(queryString) + 5 * nParams); const char **paramValues = (const char **)ckalloc(nParams * sizeof (*paramValues)); + int *paramLengths = (int *)ckalloc(nParams * sizeof (int)); const char *input = queryString; char *output = newQueryString; int paramIndex = 0; @@ -2991,9 +3008,10 @@ static int expand_parameters(Tcl_Interp *interp, const char *queryString, int nP // If the name is not present in the parameter array, then treat it as a NULL // in the SQL sense, represented by a literal NULL in the parameter list if(paramValueObj) { - paramValues[paramIndex] = Tcl_GetString(paramValueObj); + paramValues[paramIndex] = Tcl_GetStringFromObj(paramValueObj, ¶mLengths[paramIndex]); } else { paramValues[paramIndex] = NULL; + paramLengths[paramIndex] = 0; } // First param (paramValues[0]) is $1, etc... @@ -3019,6 +3037,10 @@ static int expand_parameters(Tcl_Interp *interp, const char *queryString, int nP // If this triggers then something is very wrong with the logic above. assert(paramIndex == nParams); + if (array_to_utf8(interp, paramValues, paramLengths, nParams, bufferPtr) != TCL_OK) { + goto error_return; + } + // Normal return, push parameters and return OK. *paramValuesPtr = paramValues; *newQueryStringPtr = newQueryString; @@ -3027,6 +3049,7 @@ static int expand_parameters(Tcl_Interp *interp, const char *queryString, int nP error_return: // Something went wrong, clean up and return ERROR. if(paramValues) ckfree((void *)paramValues); + if(paramLengths) ckfree((void *)paramLengths); if(newQueryString) ckfree((void *)newQueryString); return TCL_ERROR; } @@ -3203,7 +3226,7 @@ Pg_select(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) } if (nParams) { - if(expand_parameters(interp, queryString, nParams, paramArrayName, &newQueryString, ¶mValues) == TCL_ERROR) { + if(expand_parameters(interp, queryString, nParams, paramArrayName, &newQueryString, ¶mValues, ¶msBuffer) == TCL_ERROR) { return TCL_ERROR; } queryString = newQueryString; @@ -3826,7 +3849,7 @@ Pg_sendquery(ClientData cData, Tcl_Interp *interp, int objc, } if(nParams) { // After this point we must free newExecString and paramValues before exiting - if (expand_parameters(interp, execString, nParams, paramArrayName, &newExecString, ¶mValues) == TCL_ERROR) { + if (expand_parameters(interp, execString, nParams, paramArrayName, &newExecString, ¶mValues, ¶msBuffer) == TCL_ERROR) { return TCL_ERROR; } execString = newExecString; From 0a4344e732eef50a59d832a0f2cc1f87fbf80459 Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Thu, 25 Mar 2021 19:23:55 +0000 Subject: [PATCH 13/17] convert handle_substitutions --- generic/pgtclCmds.c | 8 ++++---- generic/tokenize.c | 24 ++++++++++++++++-------- generic/tokenize.h | 2 +- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index 800e679..5305a64 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -871,7 +871,7 @@ Pg_exec(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) Tcl_SetResult(interp, "-variables can not be used with positional or named parameters", TCL_STATIC); return TCL_ERROR; } - if (handle_substitutions(interp, execString, &newExecString, ¶mValues, &nParams, 1) != TCL_OK) { + if (handle_substitutions(interp, execString, &newExecString, ¶mValues, &nParams, ¶msBuffer) != TCL_OK) { return TCL_ERROR; } if(nParams) @@ -3195,7 +3195,7 @@ Pg_select(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) } if (useVariables) { - if (handle_substitutions(interp, queryString, &newQueryString, ¶mValues, &nParams, 1) != TCL_OK) { + if (handle_substitutions(interp, queryString, &newQueryString, ¶mValues, &nParams, ¶msBuffer) != TCL_OK) { return TCL_ERROR; } if(nParams) @@ -3827,13 +3827,13 @@ Pg_sendquery(ClientData cData, Tcl_Interp *interp, int objc, Tcl_SetResult(interp, "-variables can not be used with positional or named parameters", TCL_STATIC); return TCL_ERROR; } - if (handle_substitutions(interp, execString, &newExecString, ¶mValues, &nParams, 1) != TCL_OK) { + if (handle_substitutions(interp, execString, &newExecString, ¶mValues, &nParams, ¶msBuffer) != TCL_OK) { return TCL_ERROR; } if(nParams) execString = newExecString; else { // No variables being substituted, fall back to simple code path - ckfree(newExecString); + ckfree((void *)newExecString); newExecString = NULL; ckfree((void *)paramValues); paramValues = NULL; diff --git a/generic/tokenize.c b/generic/tokenize.c index 5becff9..2f88427 100644 --- a/generic/tokenize.c +++ b/generic/tokenize.c @@ -455,12 +455,15 @@ int Pg_sqlite3GetToken(const char *z, enum sqltoken *tokenType){ return i; } -int handle_substitutions(Tcl_Interp *interp, const char *sql, char **newSqlPtr, const char ***replacementArrayPtr, int *replacementArrayLengthPtr, int hardError) +extern int array_to_utf8(Tcl_Interp *interp, const char **paramValues, int *paramLengths, int nParams, const char **bufferPtr); + +int handle_substitutions(Tcl_Interp *interp, const char *sql, char **newSqlPtr, const char ***replacementArrayPtr, int *replacementArrayLengthPtr, const char **bufferPtr) { char *newSql = ckalloc(strlen(sql)+1); // Worst possible case? the sql is nothing but ":varname" and they're all one character names. This // will still be big enough. const char **replacementArray = (const char **)ckalloc((strlen(sql)/2) * (sizeof *replacementArray)); + int *lengthArray = (int *)ckalloc((strlen(sql)/2) * sizeof (int)); const char *p; char *q; @@ -475,16 +478,14 @@ int handle_substitutions(Tcl_Interp *interp, const char *sql, char **newSqlPtr, len = Pg_sqlite3GetToken(p, &tk); switch (tk) { case TK_SQLVAR: { - if(hardError || nextVarIndex) { - Tcl_SetResult(interp, "Can't combine Tcl and Postgres substitutions", TCL_STATIC); - result = TCL_ERROR; - } else { - result = TCL_CONTINUE; - } + Tcl_SetResult(interp, "Can't combine Tcl and Postgres substitutions", TCL_STATIC); + result = TCL_ERROR; goto cleanup_and_exit; } case TK_TCLVAR: { char *nameBuf = ckalloc(len); + int stringLength; + Tcl_Obj *varObj = NULL; const char *val = NULL; int i; int skip = 1; @@ -499,11 +500,13 @@ int handle_substitutions(Tcl_Interp *interp, const char *sql, char **newSqlPtr, for(i = skip; i < len; i++) nameBuf[i-skip] = p[i]; nameBuf[i-skip-trunc] = 0; - val = Tcl_GetVar(interp, nameBuf, 0); + varObj = Tcl_GetVar2Ex(interp, nameBuf, NULL, 0); + val = Tcl_GetStringFromObj(varObj, &stringLength); ckfree(nameBuf); p += len; replacementArray[nextVarIndex] = val; + lengthArray[nextVarIndex] = stringLength; sprintf(q, "$%d", nextVarIndex+1); //1 indexed while(*q) q++; nextVarIndex++; @@ -522,7 +525,12 @@ int handle_substitutions(Tcl_Interp *interp, const char *sql, char **newSqlPtr, } *q = 0; + if(result == TCL_OK) + result = array_to_utf8(interp, replacementArray, lengthArray, nextVarIndex, bufferPtr); + cleanup_and_exit: + if(lengthArray) ckfree(lengthArray); + if(result == TCL_OK) { *newSqlPtr = newSql; *replacementArrayPtr = replacementArray; diff --git a/generic/tokenize.h b/generic/tokenize.h index 6a61fd1..722bb75 100644 --- a/generic/tokenize.h +++ b/generic/tokenize.h @@ -14,7 +14,7 @@ TK_CAST, TK_HASH }; int Pg_sqlite3GetToken(const char *z, enum sqltoken *tokenType); -int handle_substitutions(Tcl_Interp *interp, const char *sql, char **newSqlPtr, const char ***replacementArrayPtr, int *replacementArrayLengthPtr, int hardError); +int handle_substitutions(Tcl_Interp *interp, const char *sql, char **newSqlPtr, const char ***replacementArrayPtr, int *replacementArrayLengthPtr, const char **bufferPtr); #define sqlite3Isdigit(x) isdigit(x) #define sqlite3Isspace(x) isspace(x) From 1c8d5793449c78dfb4609c7397d2d308ff21a481 Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Thu, 25 Mar 2021 22:58:55 +0000 Subject: [PATCH 14/17] Also convert prepared statement names. --- generic/pgtclCmds.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index 5305a64..124cbe5 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -1052,7 +1052,7 @@ Pg_exec_prepared(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST statementNameString = Tcl_GetString(objv[2]); - result = PQexecPrepared(conn, statementNameString, nParams, paramValues, NULL, NULL, 0); + result = PQexecPrepared(conn, externalString(statementNameString), nParams, paramValues, NULL, NULL, 0); if (paramValues != (const char **)NULL) { ckfree ((void *)paramValues); @@ -1527,18 +1527,17 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) */ for (tupno = 0; tupno < PQntuples(result); tupno++) { - const char *rawField0 = PGgetvalue(result, resultid->nullValueString, tupno, 0); - char *field0 = ckalloc(strlen(rawField0)*4+1); + const char *field0 = utfString(PGgetvalue(result, resultid->nullValueString, tupno, 0)); + char *dupfield0 = ckalloc(strlen(field0)+1); - strcpy(field0, utfString(rawField0)); + strcpy(dupfield0, field0); for (i = 1; i < PQnfields(result); i++) { Tcl_Obj *fieldNameObj; - fieldNameObj = Tcl_NewObj (); - Tcl_SetStringObj(fieldNameObj, field0, -1); + Tcl_SetStringObj(fieldNameObj, dupfield0, -1); Tcl_AppendToObj(fieldNameObj, ",", 1); Tcl_AppendToObj(fieldNameObj, PQfname(result, i), -1); @@ -1550,11 +1549,11 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) Tcl_NewStringObj( val, -1), TCL_LEAVE_ERR_MSG) == NULL) { Tcl_DecrRefCount(fieldNameObj); - ckfree(field0); + ckfree(dupfield0); return TCL_ERROR; } } - ckfree(field0); + ckfree(dupfield0); } return TCL_OK; } @@ -1658,10 +1657,9 @@ Pg_result(ClientData cData, Tcl_Interp *interp, int objc, Tcl_Obj *CONST objv[]) continue; } } - string = utfString(string); if (Tcl_SetVar2(interp, arrayName, PQfname(result, i), - string, + utfString(string), TCL_LEAVE_ERR_MSG) == NULL) return TCL_ERROR; } @@ -5456,7 +5454,7 @@ Pg_sql(ClientData cData, Tcl_Interp *interp, int objc, * of query */ if (prepared) { - iResult = PQsendQueryPrepared(conn, execString, count, paramValues, paramLengths, binValues, binresults); + iResult = PQsendQueryPrepared(conn, externalString(execString), count, paramValues, paramLengths, binValues, binresults); } else if (params) { iResult = PQsendQueryParams(conn, externalString(execString), count, NULL, paramValues, paramLengths, binValues, binresults); @@ -5470,7 +5468,7 @@ Pg_sql(ClientData cData, Tcl_Interp *interp, int objc, } else { if (prepared) { - result = PQexecPrepared(conn, execString, count, paramValues, paramLengths, binValues, binresults); + result = PQexecPrepared(conn, externalString(execString), count, paramValues, paramLengths, binValues, binresults); } else if (params) { result = PQexecParams(conn, externalString(execString), count, NULL, paramValues, paramLengths, binValues, binresults); } else { From afda23dbfe23a9a33429230b630370c0c5907488 Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Wed, 31 Mar 2021 18:23:19 +0000 Subject: [PATCH 15/17] Return failure if can't initialise utf-8 encoding. --- generic/pgtcl.c | 5 +++-- generic/pgtclCmds.c | 6 +++++- generic/pgtclCmds.h | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/generic/pgtcl.c b/generic/pgtcl.c index 41781ee..f636e61 100644 --- a/generic/pgtcl.c +++ b/generic/pgtcl.c @@ -160,7 +160,7 @@ Pgtcl_Init(Tcl_Interp *interp) * No really good way to do error handling here, since we * don't know how we were loaded */ - return FALSE; + return TCL_ERROR; } #endif @@ -181,7 +181,8 @@ Pgtcl_Init(Tcl_Interp *interp) if (tclversion >= 8.1) Tcl_PutEnv("PGCLIENTENCODING=UNICODE"); - pgtclInitEncoding(interp); + if(pgtclInitEncoding(interp) != TCL_OK) + return TCL_ERROR; /* register all pgtcl commands */ diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index 124cbe5..d274c7d 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -247,8 +247,12 @@ static Tcl_Encoding utf8encoding = NULL; /* * Initialize utf8encoding */ -void pgtclInitEncoding(Tcl_Interp *interp) { +int pgtclInitEncoding(Tcl_Interp *interp) { utf8encoding = Tcl_GetEncoding(interp, "utf-8"); + if (utf8encoding != NULL) + return TCL_OK; + Tcl_SetResult(interp, "Could not initialize encoding utf-8", TCL_STATIC); + return TCL_ERROR; } // The following two functions "waste" a DStrings storage by not freeing it until it's needed again diff --git a/generic/pgtclCmds.h b/generic/pgtclCmds.h index e93d40b..57d394a 100644 --- a/generic/pgtclCmds.h +++ b/generic/pgtclCmds.h @@ -17,7 +17,7 @@ #include #include "libpq-fe.h" -extern void pgtclInitEncoding(Tcl_Interp *interp); +extern int pgtclInitEncoding(Tcl_Interp *interp); /* MOVED structure definitions for connection IDs to pctclId.h */ From 9c48fb32481f6a5406bb82f4d3ed209bcb5a1ae5 Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Wed, 31 Mar 2021 18:25:32 +0000 Subject: [PATCH 16/17] Tcl_GetEncoding already sets error message. --- generic/pgtclCmds.c | 1 - 1 file changed, 1 deletion(-) diff --git a/generic/pgtclCmds.c b/generic/pgtclCmds.c index d274c7d..c1bd8c6 100644 --- a/generic/pgtclCmds.c +++ b/generic/pgtclCmds.c @@ -251,7 +251,6 @@ int pgtclInitEncoding(Tcl_Interp *interp) { utf8encoding = Tcl_GetEncoding(interp, "utf-8"); if (utf8encoding != NULL) return TCL_OK; - Tcl_SetResult(interp, "Could not initialize encoding utf-8", TCL_STATIC); return TCL_ERROR; } From 94bc3d5e55f81a0f40246c23653bd833e98b8dde Mon Sep 17 00:00:00 2001 From: Peter da Silva Date: Thu, 1 Apr 2021 19:44:22 +0000 Subject: [PATCH 17/17] Add tests for high unicode --- configure.in | 2 +- tests/pgtcl.test | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/configure.in b/configure.in index 500565a..0a4adc1 100755 --- a/configure.in +++ b/configure.in @@ -19,7 +19,7 @@ dnl to configure the system for the local environment. # so you can encode the package version directly into the source files. #----------------------------------------------------------------------- -AC_INIT([pgtcl], [2.7.5]) +AC_INIT([pgtcl], [2.7.6]) #----- # Version with patch stripped diff --git a/tests/pgtcl.test b/tests/pgtcl.test index 6926ff8..2f9e764 100644 --- a/tests/pgtcl.test +++ b/tests/pgtcl.test @@ -1259,4 +1259,56 @@ test pgtcl-10.3 {copy out} -body { set result } -result {{name batfink} {studio {Hal Seeger Studios}} {type superhero} {wings steel}} +# +# +# +test pgtcl-11.0 {using pg_exec with 4-byte unicode} -body { + + unset -nocomplain res + + set conn [pg::connect -connlist [array get ::conninfo]] + + set glyph "𝔄" + + set res [$conn exec {SELECT relname + FROM Pg_class + WHERE relname LIKE '$glyph'}] + + set results [pg::result $res -list] + + pg_result $res -clear + + pg_disconnect $conn + + lsort $results + +} -result [list] + +# +# +# +test pgtcl-11.1 {using pg_exec with 4-byte unicode variables} -body { + + unset -nocomplain res + + set conn [pg::connect -connlist [array get ::conninfo]] + + set glyph "𝔄" + + set res [$conn exec -variables {SELECT relname + FROM Pg_class + WHERE relname LIKE :glyph}] + + set results [pg::result $res -list] + + pg_result $res -clear + + pg_disconnect $conn + + lsort $results + +} -result [list] + + + puts "tests complete"