[AOLSERVER] nspostgres - patch

[Majid Khan]

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.

[Jim]

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 <majid...@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;
> *}

> *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
aolserv...@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/aolserver-talk

[Agustin Lopez]

Hi all!

Talking about nspostgres, has anybody implemented any way to auto reconnect to the database in a Postgresql start / stop?

Thanks,
Agustin

Jim <jim.p...@gmail.com> escribió:

--
Enviado desde mi teléfono Android con K-9 Mail. Disculpa mi brevedad

[Majid Khan]

Hi Jim,

 

I saw your code, looks good and covered all my concerns.

 

Majid

[Jim]

Heya Majid,

Thanks for looking; I also wonder if you could test it as well... need
to know if it builds on your platform, and if that function works...
should be much more solid now, as the else clauses only run if not
error.

To all: I hope you can also test the code; you can:

git clone git://github.com/jwlynch/nspostgres.git

which is not the main repo, it's my repo with the changes I made.

-Jim

On 2/28/12, Majid Khan <majid...@gmail.com> wrote:
> Hi Jim,
>
> I saw your code, looks good and covered all my concerns.
>
> Majid

------------------------------------------------------------------------------

[Jeff Rogers]

Majid,

Can you come up with a code sample or use case that demonstrates the
buggy behavior? That's the best way to show that it actually is a
problem, and that the changes fix the problem.

-J

Majid Khan 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;
> *}

> *close(fd);
> * return TCL_ERROR;
> }
> Regards,
> Majid.
>
>
>
> ------------------------------------------------------------------------------

> Virtualization& Cloud Management Using Capacity Planning

[Brad Chick]

I was wondering if anyone has had any luck building the nsoracle driver
against Oracle Instant Client, rather than the full oracle client libs?

Thanks

--
==============================
BRAD CHICK
==============================

Br...@ChickCentral.com
734.662.1701 (h)
734.646.9372 (m)

"Make Some Time for Wasting!"
_
| |
___| |__ ___ ___ _ __ ___
/ __| '_ \ / _ \/ _ \ '__/ __|
(__| | | | __/ __/ | \__ \
\___|_| |_|\___|\___|_| |___/
================================

[Jim]

Heya Brad,

It's going to be more efficient of your own time to be very
informative and specific of your situation wrt building. Any logs you
have, what parts of oracle you've installed, etc.

Personally, I do not use oracle at all, so (other than this request
for more information) I may not be able to help you specifically, and
nor will others before you present your full, complete, detailed
situation. One question comes to mind, have you installed the headers
and libs from oracle instant client, and have you provided the path to
the includes and path to the libs, to nsoracle, presumably by editing
its makefile?

Others, what operating system are you running? what compiler? which
version? A list of things you have tried, with results and logs for
each?

[John Caruso]

On Wednesday 11:37 AM 2/29/2012, Brad Chick wrote:
>I was wondering if anyone has had any luck building the nsoracle driver
>against Oracle Instant Client, rather than the full oracle client libs?

Yes, we've been doing this for years on Redhat Enterprise Linux and have done it on OS X as well. You'll need both the instantclient-basic[lite] package and the instantclient-sdk package (the latter to get the include files) for building, though you only need the former at run time. We do a few things to make OIC work better with calling packages:

1) Create a symlink from rdbms/demo to sdk/include (nsoracle uses the former for include files)

2) Create the following symlinks under the lib/ directory:
ln -s libclntsh.so.* libclntsh.so
ln -s libocci.so.* libocci.so

I believe the second was required more for OS X than Linux, but there's no harm either way.

- John