Re: [HACKERS] [GENERAL] Notify argument?
Fix applied. --- Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: The breakage will come when we lengthen NAMEDATALEN, which I plan to tackle for 7.3. We will need to re-order the NOTIFY structure and put the NAMEDATALEN string at the end of the struct so differing namedatalen backend/clients will work. If you want to break it, 7.3 would probably be the time to do it. :-) Users will need a recompile pre-7.3 to use notify for 7.3 and later anyway. If we're going to change the structure anyway, let's fix it to be independent of NAMEDATALEN. Instead of charrelname[NAMEDATALEN]; int be_pid; let's do char *relname; int be_pid; This should require no source-level changes in calling C code, thanks to C's equivalence between pointers and arrays. We can preserve the fact that freeing a PQnotifies result takes only one free() with a little hacking to make the string be allocated in the same malloc call: newNotify = (PGnotify *) malloc(sizeof(PGnotify) + strlen(str) + 1); newNotify-relname = (char *) newNotify + sizeof(PGnotify); strcpy(newNotify-relname, str); Thus, with one line of extra ugliness inside the library, we solve the problem permanently. regards, tom lane -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup.| Drexel Hill, Pennsylvania 19026 ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] [GENERAL] Notify argument?
Here is a patch that implements Tom's suggestion of mallocing the
relation name string as part of PQnotify and not depending on
NAMEDATALEN. Nice trick.
---
Tom Lane wrote:
Bruce Momjian [EMAIL PROTECTED] writes:
The breakage will come when we lengthen NAMEDATALEN, which I plan to
tackle for 7.3. We will need to re-order the NOTIFY structure and put
the NAMEDATALEN string at the end of the struct so differing namedatalen
backend/clients will work. If you want to break it, 7.3 would probably
be the time to do it. :-) Users will need a recompile pre-7.3 to use
notify for 7.3 and later anyway.
If we're going to change the structure anyway, let's fix it to be
independent of NAMEDATALEN. Instead of
charrelname[NAMEDATALEN];
int be_pid;
let's do
char *relname;
int be_pid;
This should require no source-level changes in calling C code, thanks
to C's equivalence between pointers and arrays. We can preserve the
fact that freeing a PQnotifies result takes only one free() with a
little hacking to make the string be allocated in the same malloc call:
newNotify = (PGnotify *) malloc(sizeof(PGnotify) + strlen(str) + 1);
newNotify-relname = (char *) newNotify + sizeof(PGnotify);
strcpy(newNotify-relname, str);
Thus, with one line of extra ugliness inside the library, we solve the
problem permanently.
regards, tom lane
--
Bruce Momjian| http://candle.pha.pa.us
[EMAIL PROTECTED] | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup.| Drexel Hill, Pennsylvania 19026
Index: src/interfaces/libpq/fe-exec.c
===
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.118
diff -c -r1.118 fe-exec.c
*** src/interfaces/libpq/fe-exec.c 8 Apr 2002 03:48:10 - 1.118
--- src/interfaces/libpq/fe-exec.c 15 Apr 2002 00:15:29 -
***
*** 1510,1517
return EOF;
if (pqGets(conn-workBuffer, conn))
return EOF;
! newNotify = (PGnotify *) malloc(sizeof(PGnotify));
! strncpy(newNotify-relname, conn-workBuffer.data, NAMEDATALEN);
newNotify-be_pid = be_pid;
DLAddTail(conn-notifyList, DLNewElem(newNotify));
return 0;
--- 1510,1525
return EOF;
if (pqGets(conn-workBuffer, conn))
return EOF;
!
! /*
!* Store the relation name right after the PQnotify structure so it can
!* all be freed at once. We don't use NAMEDATALEN because we don't
!* want to tie this interface to a specific server name length.
!*/
! newNotify = (PGnotify *) malloc(sizeof(PGnotify) +
! strlen(conn-workBuffer.data) + 1);
! newNotify-relname = (char *)newNotify + sizeof(PGnotify);
! strcpy(newNotify-relname, conn-workBuffer.data);
newNotify-be_pid = be_pid;
DLAddTail(conn-notifyList, DLNewElem(newNotify));
return 0;
Index: src/interfaces/libpq/libpq-fe.h
===
RCS file: /cvsroot/pgsql/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.83
diff -c -r1.83 libpq-fe.h
*** src/interfaces/libpq/libpq-fe.h 5 Mar 2002 06:07:26 - 1.83
--- src/interfaces/libpq/libpq-fe.h 15 Apr 2002 00:15:40 -
***
*** 105,112
*/
typedef struct pgNotify
{
! charrelname[NAMEDATALEN]; /* name of relation containing
!*
data */
int be_pid; /* process id of backend */
} PGnotify;
--- 105,111
*/
typedef struct pgNotify
{
! char*relname; /* name of relation containing data */
int be_pid; /* process id of backend */
} PGnotify;
---(end of broadcast)---
TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] [GENERAL] Notify argument?
Neil Conway wrote: On Wed, Mar 20, 2002 at 04:10:14PM -0500, Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: The breakage will come when we lengthen NAMEDATALEN, which I plan to tackle for 7.3. We will need to re-order the NOTIFY structure and put the NAMEDATALEN string at the end of the struct so differing namedatalen backend/clients will work. If you want to break it, 7.3 would probably be the time to do it. :-) Users will need a recompile pre-7.3 to use notify for 7.3 and later anyway. If we're going to change the structure anyway, let's fix it to be independent of NAMEDATALEN. Sounds good. If we're making other backwards-incompatible changes to pgNotify, one thing that bugs me about the API is the use of relname to refer to name of the NOTIFY/LISTEN condition. This is incorrect because the name of a condition is _not_ the name of a relation -- there is no necessary correspondence between these. The names of NOTIFY conditions are arbitrary, and chosen by the application developer. The same terminology is used in the backend NOTIFY/LISTEN code (e.g. pg_listener), but the major source of incompatibility will be the change to libpq. Would it be a good idea to rename relname to something more sensible? Renaming the column would make an API change. I was talking only about requiring a recompile. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 853-3000 + If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup.| Drexel Hill, Pennsylvania 19026 ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [GENERAL] Notify argument?
[EMAIL PROTECTED] (Neil Conway) writes: If we're going to change the structure anyway, let's fix it to be independent of NAMEDATALEN. Sounds good. If we're making other backwards-incompatible changes to pgNotify, one thing that bugs me about the API is the use of relname to refer to name of the NOTIFY/LISTEN condition. I hear you ... but my proposal only requires a recompile, *not* any source code changes. Renaming the field would break client source code. I doubt it's worth that. regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [HACKERS] [GENERAL] Notify argument?
On Thu, 2002-03-21 at 00:16, Tom Lane wrote: [EMAIL PROTECTED] (Neil Conway) writes: If we're going to change the structure anyway, let's fix it to be independent of NAMEDATALEN. Sounds good. If we're making other backwards-incompatible changes to pgNotify, one thing that bugs me about the API is the use of relname to refer to name of the NOTIFY/LISTEN condition. I hear you ... but my proposal only requires a recompile, *not* any source code changes. Renaming the field would break client source code. I doubt it's worth that. Okay, that's fair -- I'll leave it as it is. Cheers, Neil -- Neil Conway [EMAIL PROTECTED] PGP Key ID: DB3C29FC ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/users-lounge/docs/faq.html
Re: [HACKERS] [GENERAL] Notify argument?
Bruce Momjian [EMAIL PROTECTED] writes: The breakage will come when we lengthen NAMEDATALEN, which I plan to tackle for 7.3. We will need to re-order the NOTIFY structure and put the NAMEDATALEN string at the end of the struct so differing namedatalen backend/clients will work. If you want to break it, 7.3 would probably be the time to do it. :-) Users will need a recompile pre-7.3 to use notify for 7.3 and later anyway. If we're going to change the structure anyway, let's fix it to be independent of NAMEDATALEN. Instead of charrelname[NAMEDATALEN]; int be_pid; let's do char *relname; int be_pid; This should require no source-level changes in calling C code, thanks to C's equivalence between pointers and arrays. We can preserve the fact that freeing a PQnotifies result takes only one free() with a little hacking to make the string be allocated in the same malloc call: newNotify = (PGnotify *) malloc(sizeof(PGnotify) + strlen(str) + 1); newNotify-relname = (char *) newNotify + sizeof(PGnotify); strcpy(newNotify-relname, str); Thus, with one line of extra ugliness inside the library, we solve the problem permanently. regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
