Hi Jim,
I saw your code, looks good and covered all my concerns.
Majid
On Fri, Feb 24, 2012 at 6:06 PM, Jim <jim.play...@gmail.com> wrote:
> 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
>
------------------------------------------------------------------------------
Keep Your Developer Skills Current with LearnDevNow!
The most comprehensive online learning library for Microsoft developers
is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
Metro Style Apps, more. Free future releases when you subscribe now!
http://p.sf.net/sfu/learndevnow-d2d
_______________________________________________
aolserver-talk mailing list
aolserver-talk@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/aolserver-talk
