Re: [HACKERS] [PATCH] Add use of asprintf()

2013-10-15 Thread Asif Naeem
On Tue, Oct 15, 2013 at 10:55 AM, David Rowley dgrowle...@gmail.com wrote: Though this is not yet enough to get the windows build work with me... I'm still getting link failures for isolationtester.c D:\Postgres\c\pgsql.sln (default target) (1) - D:\Postgres\c\isolationtester.vcxproj

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-10-15 Thread David Rowley
On Tue, Oct 15, 2013 at 8:00 PM, Asif Naeem anaeem...@gmail.com wrote: On Tue, Oct 15, 2013 at 10:55 AM, David Rowley dgrowle...@gmail.comwrote: Though this is not yet enough to get the windows build work with me... I'm still getting link failures for isolationtester.c

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-10-14 Thread David Rowley
On Sat, Oct 12, 2013 at 2:58 AM, Alvaro Herrera alvhe...@2ndquadrant.comwrote: Peter Eisentraut escribió: On Tue, 2013-09-17 at 15:13 +0500, Asif Naeem wrote: I did put some time review the patch, please see my findings below i.e. Updated patch for this. Looks good to me.

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-10-14 Thread David Rowley
On Mon, Oct 14, 2013 at 9:45 PM, David Rowley dgrowle...@gmail.com wrote: On Sat, Oct 12, 2013 at 2:58 AM, Alvaro Herrera alvhe...@2ndquadrant.comwrote: Peter Eisentraut escribió: On Tue, 2013-09-17 at 15:13 +0500, Asif Naeem wrote: I did put some time review the patch, please see my

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-10-14 Thread Peter Eisentraut
On Mon, 2013-10-14 at 23:08 +1300, David Rowley wrote: Looks like something like: #ifndef WIN32 #define HAVE_VA_COPY 1 #endif would need to be added to asprintf.c, but also some work needs to be done with mcxt.c as it uses va_copy unconditionally. Perhaps just defining a macro

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-10-14 Thread Amit Kapila
On Tue, Oct 15, 2013 at 2:18 AM, Peter Eisentraut pete...@gmx.net wrote: On Mon, 2013-10-14 at 23:08 +1300, David Rowley wrote: Looks like something like: #ifndef WIN32 #define HAVE_VA_COPY 1 #endif would need to be added to asprintf.c, but also some work needs to be done with mcxt.c

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-10-14 Thread Asif Naeem
+1 I think you can safely use va_list without copy on Windows. va_copy is available in Visual Studio 2013 as part of support for C99, previous versions don't have it. Regards, Muhammad Asif Naeem On Tue, Oct 15, 2013 at 10:33 AM, Amit Kapila amit.kapil...@gmail.comwrote: On Tue, Oct 15, 2013

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-10-14 Thread David Rowley
On Tue, Oct 15, 2013 at 9:48 AM, Peter Eisentraut pete...@gmx.net wrote: On Mon, 2013-10-14 at 23:08 +1300, David Rowley wrote: Looks like something like: #ifndef WIN32 #define HAVE_VA_COPY 1 #endif would need to be added to asprintf.c, but also some work needs to be done

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-10-11 Thread Alvaro Herrera
Peter Eisentraut escribió: On Tue, 2013-09-17 at 15:13 +0500, Asif Naeem wrote: I did put some time review the patch, please see my findings below i.e. Updated patch for this. Looks good to me. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-10-03 Thread Asif Naeem
You are right, wget worked. Latest patch looks good to me. make check run fine. Thank you Peter. On Wed, Oct 2, 2013 at 5:02 PM, Peter Eisentraut pete...@gmx.net wrote: On 10/2/13 5:12 AM, Asif Naeem wrote: Neither git nor patch command apply the patch successfully. Can you please guide

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-10-02 Thread Asif Naeem
Thank you Peter. When I try to apply the patch on latest PG source code on master branch, it gives the following error message i.e. asif$ git apply /Users/asif/core/Patch\:\ Add\ use\ of\ asprintf\(\)/v2-0001-Add-use-of-asprintf.patch /Users/asif/core/Patch: Add use of

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-10-02 Thread Peter Eisentraut
On 10/2/13 5:12 AM, Asif Naeem wrote: Neither git nor patch command apply the patch successfully. Can you please guide ?. Thanks. Works for me. Check that your email client isn't mangling line endings. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-09-21 Thread Peter Eisentraut
On Mon, 2013-09-16 at 17:31 -0300, Alvaro Herrera wrote: Looks good to me, except that pg_asprintf seems to be checking ret instead of rc. Ah, good catch! Is there a reason for the API discrepancy of pg_asprintf vs. psprintf? I don't see that we use the integer return value anywhere.

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-09-21 Thread Peter Eisentraut
On Tue, 2013-09-17 at 15:13 +0500, Asif Naeem wrote: 1. It seems that you have used strdup() on multiple places in the patch, e.g. in the below code snippet is it going to lead crash if newp-ident is NULL because of strdup() failure ? static EPlan * find_plan(char *ident,

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-09-17 Thread Asif Naeem
Hi, I did put some time review the patch, please see my findings below i.e. 1. It seems that you have used strdup() on multiple places in the patch, e.g. in the below code snippet is it going to lead crash if newp-ident is NULL because of strdup() failure ? static EPlan * find_plan(char

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-09-17 Thread Asif Naeem
On Tue, Sep 17, 2013 at 3:13 PM, Asif Naeem anaeem...@gmail.com wrote: Hi, I did put some time review the patch, please see my findings below i.e. 1. It seems that you have used strdup() on multiple places in the patch, e.g. in the below code snippet is it going to lead crash if newp-ident

Re: [HACKERS] [PATCH] Add use of asprintf()

2013-09-16 Thread Alvaro Herrera
Peter Eisentraut wrote: The attached patch should speak for itself. Yeah, it's a very nice cleanup. I have supplied a few variants: - asprintf() is the standard function, supplied by libpgport if necessary. - pg_asprintf() is asprintf() with automatic error handling (like pg_malloc(),