Re: pgAgent: C++ Port - Patch Review

Hi,

On Thu, Oct 24, 2013 at 3:03 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:

> On Thu, Oct 24, 2013 at 10:29 AM, Linreg <linreg(at)gmx(dot)net> wrote:
> > Am Donnerstag, 24. Oktober 2013, 11:11:21 schrieb Neel Patel:
> >
> >> Hi,
> >
> >>
> >
> >> Below are the review comments. Compile and tested in Linux.
> >
> >>
> >
> >> 1) During compilation we got the following error in "connection.h"
> >
> >>
> >
> >> error: pgsql/libpq-fe.h: No such file or directory
> >
> >>
> >
> >> To solve the above error we changed "#include <libpg-fe.h>" instead of
> >
> >> "#include <pgsql/libpq-fe.h>" and it is working fine. Correct me if it
> is
> >
> >> some configuration issue.
> >
> >>
> >
> >> 2) We are getting the below warning in unix.cpp file which needs to fix.
> >
> >>
> >
> >> warning: the use of `tmpnam' is dangerous, better use `mkstemp'.
> >
> >>
> >
> >>
> >
> >> 3) Some of the code is commented and not used so remove the unnecessary
> >
> >> code.
> >
> >>
> >
> >> 4) README file should be modified according to new changes.
> >
> >>
> >
> >> 5) In Execute() method in Job.cpp file, we should have to delete "steps"
> >
> >> from wherever we are returning otherwise it will create the memory leak.
> >
> >>
> >
> >>
> >
> >> Thanks,
> >
> >> Neel Patel
> >
> >
> >
> > 1.)
> >
> > it is system dependent. On my SUSE system pibpg-header to be installed
> under
> > the pgsql directory.
> >
> > Example: /usr/include/pgsql/libpq-fe.h
>
> What did the previous code do? That worked. Typically we would not
> expect to see a pgsql/ prefix on a header, but would ensure the
> compiler is passed the include directory with that directory in it.
>

Previous code is without pgsql/ prefix on a header. So i think we should
have only "#include <libpg-fe.h>".

>
> --
> Dave Page
> Blog: http://pgsnake.blogspot.com
> Twitter: @pgsnake
>
> EnterpriseDB UK: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>