On Mon, Jun 8, 2015 at 9:22 PM, Michael Meskes wrote:
> On Mon, Jun 08, 2015 at 01:57:32PM +0900, Michael Paquier wrote:
>> diff --git a/src/interfaces/ecpg/compatlib/informix.c
>> b/src/interfaces/ecpg/compatlib/informix.c
>> index d6de3ea..c1e3dfb 100644
>> --- a/src/interfaces/ecpg/compatlib/informix.c
>> +++ b/src/interfaces/ecpg/compatlib/informix.c
>> @@ -672,6 +672,7 @@ intoasc(interval * i, char *str)
>> if (!str)
>> return -errno;
>>
>> + free(str);
>> return 0;
>> }
>
> This function never worked it seems, we need to use a temp string, copy it to
> str and then free the temp one.
And the caller needs to be sure that str actually allocates enough
space.. That's not directly ECPG's business, still it feels
uncomfortable. I fixed it with the attached.
>> diff --git a/src/interfaces/ecpg/ecpglib/connect.c
>> b/src/interfaces/ecpg/ecpglib/connect.c
>> index 55c5680..12f445d 100644
>> --- a/src/interfaces/ecpg/ecpglib/connect.c
>> +++ b/src/interfaces/ecpg/ecpglib/connect.c
>> @@ -298,7 +298,8 @@ ECPGconnect(int lineno, int c, const char *name, const
>> char *user, const char *p
>> envname = getenv("PG_DBPATH");
>> if (envname)
>> {
>> - ecpg_free(dbname);
>> + if (dbname)
>> + ecpg_free(dbname);
>> dbname = ecpg_strdup(envname, lineno);
>> }
>
> This is superfluous, at least with glibc. free()'s manpage clearly says "If
> ptr is NULL, no operation is performed.", so why should we check again? Or do
> we have architectures where free(NULL) is not a noop?
The world is full of surprises, and even if free(NULL) is a noop on
modern platforms, I would rather take it defensively and check for a
NULL pointer as Postgres supports many platforms. Other code paths
tend to do already so, per se for example ECPGconnect.
>> diff --git a/src/interfaces/ecpg/preproc/descriptor.c
>> b/src/interfaces/ecpg/preproc/descriptor.c
>> index 053a7af..ebd95d3 100644
>> --- a/src/interfaces/ecpg/preproc/descriptor.c
>> +++ b/src/interfaces/ecpg/preproc/descriptor.c
>
> Do we really have to worry about these in a short running application like a
> precompiler, particularly if they add more than a simple free() command?
Perhaps I am overdoing it. I would have them for correctness (some
other code paths do so) but if you think that the impact is minimal
there then I am fine to not modify descriptor.c.
> But then, you already did the work, so why not. Anyway, please tell me if you
> want to update the patch or if I should commit whatever is clear already.
A new patch is attached.
Regards,
--
Michael
diff --git a/src/interfaces/ecpg/compatlib/informix.c b/src/interfaces/ecpg/compatlib/informix.c
index d6de3ea..8d81c83 100644
--- a/src/interfaces/ecpg/compatlib/informix.c
+++ b/src/interfaces/ecpg/compatlib/informix.c
@@ -666,12 +666,16 @@ dttofmtasc(timestamp * ts, char *output, int str_len, char *fmtstr)
int
intoasc(interval * i, char *str)
{
+ char *tmp;
+
errno = 0;
- str = PGTYPESinterval_to_asc(i);
+ tmp = PGTYPESinterval_to_asc(i);
- if (!str)
+ if (!tmp)
return -errno;
+ memcpy(str, tmp, strlen(tmp));
+ free(tmp);
return 0;
}
diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c
index 55c5680..12f445d 100644
--- a/src/interfaces/ecpg/ecpglib/connect.c
+++ b/src/interfaces/ecpg/ecpglib/connect.c
@@ -298,7 +298,8 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
envname = getenv("PG_DBPATH");
if (envname)
{
- ecpg_free(dbname);
+ if (dbname)
+ ecpg_free(dbname);
dbname = ecpg_strdup(envname, lineno);
}
@@ -314,14 +315,19 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
/* check if the identifier is unique */
if (ecpg_get_connection(connection_name))
{
- ecpg_free(dbname);
+ if (dbname)
+ ecpg_free(dbname);
ecpg_log("ECPGconnect: connection identifier %s is already in use\n",
connection_name);
return false;
}
if ((this = (struct connection *) ecpg_alloc(sizeof(struct connection), lineno)) == NULL)
+ {
+ if (dbname)
+ ecpg_free(dbname);
return false;
+ }
if (dbname != NULL)
{
diff --git a/src/interfaces/ecpg/preproc/descriptor.c b/src/interfaces/ecpg/preproc/descriptor.c
index 053a7af..ebd95d3 100644
--- a/src/interfaces/ecpg/preproc/descriptor.c
+++ b/src/interfaces/ecpg/preproc/descriptor.c
@@ -175,6 +175,7 @@ output_get_descr(char *desc_name, char *index)
for (results = assignments; results != NULL; results = results->next)
{
const struct variable *v = find_variable(results->variable);
+ char *str_zero = mm_strdup("0");
switch (results->value)
{
@@ -188,7 +189,8 @@ output_get_descr(char *desc_name, char *index)
break;
}
fprintf(yyout, "%s,", get_dtype(results->value));
- ECPGdump_a_type(yyout, v->name, v->type, v->brace_level, NULL, NULL, -1, NULL, NULL, mm_strdup("0"), NULL, NULL);
+ ECPGdump_a_type(yyout, v->name, v->type, v->brace_level, NULL, NULL, -1, NULL, NULL, str_zero, NULL, NULL);
+ free(str_zero);
}
drop_assignments();
fputs("ECPGd_EODT);\n", yyout);
@@ -292,8 +294,12 @@ output_set_descr(char *desc_name, char *index)
case ECPGd_indicator:
case ECPGd_length:
case ECPGd_type:
- fprintf(yyout, "%s,", get_dtype(results->value));
- ECPGdump_a_type(yyout, v->name, v->type, v->brace_level, NULL, NULL, -1, NULL, NULL, mm_strdup("0"), NULL, NULL);
+ {
+ char *str_zero = mm_strdup("0");
+ fprintf(yyout, "%s,", get_dtype(results->value));
+ ECPGdump_a_type(yyout, v->name, v->type, v->brace_level, NULL, NULL, -1, NULL, NULL, str_zero, NULL, NULL);
+ free(str_zero);
+ }
break;
default:
diff --git a/src/interfaces/ecpg/preproc/pgc.l b/src/interfaces/ecpg/preproc/pgc.l
index c70f298..0453409 100644
--- a/src/interfaces/ecpg/preproc/pgc.l
+++ b/src/interfaces/ecpg/preproc/pgc.l
@@ -27,7 +27,7 @@
extern YYSTYPE yylval;
static int xcdepth = 0; /* depth of nesting in slash-star comments */
-static char *dolqstart; /* current $foo$ quote start string */
+static char *dolqstart = NULL; /* current $foo$ quote start string */
static YY_BUFFER_STATE scanbufhandle;
static char *scanbuf;
@@ -550,6 +550,8 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})(.*\\{space})*.
}
<SQL>{dolqdelim} {
token_start = yytext;
+ if (dolqstart)
+ free(dolqstart);
dolqstart = mm_strdup(yytext);
BEGIN(xdolq);
startlit();
diff --git a/src/interfaces/ecpg/preproc/variable.c b/src/interfaces/ecpg/preproc/variable.c
index 2ad7b5d..bb4b664 100644
--- a/src/interfaces/ecpg/preproc/variable.c
+++ b/src/interfaces/ecpg/preproc/variable.c
@@ -437,11 +437,13 @@ remove_variable_from_list(struct arguments ** list, struct variable * var)
void
dump_variables(struct arguments * list, int mode)
{
- char *str_zero = mm_strdup("0");
+ char *str_zero;
if (list == NULL)
return;
+ str_zero = mm_strdup("0");
+
/*
* The list is build up from the beginning so lets first dump the end of
* the list:
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
