Hi Majid and the group,
I've been taking care of nspostgres and I made a git copy on github
with my changes. I did things a little differently, I think a function
should only have one exit point. So, the structure is a little
different (I added "else" clauses for "if"s that test for errors, also
a variable "result" which holds the function return value, a "break"
statement to get out of the blob loop if a db error happens, a "return
result" at the end and "result = ..." where appropriate. I also took
care of the close(fd) thing.)
Take a look! You can:
git clone git://github.com/jwlynch/nspostgres.git
Whoever wants to (would appreciate it if Dossy and Majid) can take a
look and see if this thing works; I'll ask openacs people to check it
out as well.
-Jim
On 2/6/12, Majid Khan <majidkha...@gmail.com> wrote:
> Hello All,
>
> Just checking the code of nspostgres driver (downloaded from git) and found
> that the code needs some fixes. Couple of thing which I see
>
> 1- in nspostgres.c
>
> fucntion: static int blob_dml_file
> where we see these lines:
>
> if (fd == -1)
> {
> Ns_Log (Error, " Error opening file %s: %d(%s)", filename, errno,
> strerror(errno));
> Tcl_AppendResult (interp, "can't open file ", filename, " for reading. ",
> "received error ", strerror(errno), NULL);
> }
>
> Why to continue the process when we are unable to open the file? Shouldn't
> it be
>
> if (fd == -1)
> {
> Ns_Log (Error, " Error opening file %s: %d(%s)", filename, errno,
> strerror(errno));
> Tcl_AppendResult (interp, "can't open file ", filename, " for reading. ",
> "received error ", strerror(errno), NULL);
> *return TCL_ERROR;
> *}
>
> 2- In the same function where we have this:
>
> if (Ns_PgExec(handle, query) != NS_DML) {
> Tcl_DString errString;
> Tcl_DStringInit(&errString);
> Tcl_DStringAppend
> (&errString, "Error inserting data into BLOB\n", -1);
> if(handle->verbose)
> {
> append_PQresultStatus(&errString, nspgConn->res);
>
> Tcl_DStringAppend(&errString, "SQL: ", -1);
> Tcl_DStringAppend(&errString, query, -1);
> }
> Tcl_AppendResult(interp, Tcl_DStringValue(&errString), NULL);
> Tcl_DStringFree(&errString);
> return TCL_ERROR;
> }
>
> here while returning error we are not closing the file descriptor so I see
> a resource leak so it should be
>
> if (Ns_PgExec(handle, query) != NS_DML) {
> Tcl_DString errString;
> Tcl_DStringInit(&errString);
> Tcl_DStringAppend
> (&errString, "Error inserting data into BLOB\n", -1);
> if(handle->verbose)
> {
> append_PQresultStatus(&errString, nspgConn->res);
>
> Tcl_DStringAppend(&errString, "SQL: ", -1);
> Tcl_DStringAppend(&errString, query, -1);
> }
> Tcl_AppendResult(interp, Tcl_DStringValue(&errString), NULL);
> Tcl_DStringFree(&errString);
> *close(fd);
> * return TCL_ERROR;
> }
>
>
> Regards,
>
> Majid.
>
------------------------------------------------------------------------------
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
_______________________________________________
aolserver-talk mailing list
aolserver-talk@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/aolserver-talk
