Re: [GENERAL] pg crashing - Mailing list pgsql-hackers

[moving over to hackers]

Tom Lane wrote:
> BTW, just looking at win32_shmem.c ...
>
>     retptr = malloc(bufsize + 1 + 18);    /* 1 NULL and 18 for
>                                            * Global\PostgreSQL: */
>     if (retptr == NULL)
>         elog(FATAL, "could not allocate memory for shared memory name");
>
>     strcpy(retptr, "Global\\PostgreSQL:");
>     r = GetFullPathName(DataDir, bufsize, retptr + 11, NULL);
>
> Surely that "11" ought to be "18".  Also, since the loop immediately

Yes. Very true. It still *works*, since it guts off on the proper side
of the \, but it still makes sense.

> below this is going to convert \ to /, wouldn't it be clearer to
> describe the path prefix as Global/PostgreSQL: in the first place?

Eh, that shows another bug I think. It should *not* convert the \ in
"Global\", because that one is is interpreted by the Win32 API call!

I think it should be per this patch. Seems right?


> (BTW, as far as I can tell the +1 added to the malloc request is
> useless: bufsize includes the trailing null, and the code would
> not work if it did not.)

Yeah, also true.

//Magnus
Index: win32_shmem.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/port/win32_shmem.c,v
retrieving revision 1.4
diff -c -r1.4 win32_shmem.c
*** win32_shmem.c    1 Jan 2008 19:45:51 -0000    1.4
--- win32_shmem.c    2 Jul 2008 16:44:40 -0000
***************
*** 47,64 ****
          elog(FATAL, "could not get size for full pathname of datadir %s: %lu",
               DataDir, GetLastError());

!     retptr = malloc(bufsize + 1 + 18);    /* 1 NULL and 18 for
                                           * Global\PostgreSQL: */
      if (retptr == NULL)
          elog(FATAL, "could not allocate memory for shared memory name");

      strcpy(retptr, "Global\\PostgreSQL:");
!     r = GetFullPathName(DataDir, bufsize, retptr + 11, NULL);
      if (r == 0 || r > bufsize)
          elog(FATAL, "could not generate full pathname for datadir %s: %lu",
               DataDir, GetLastError());

!     for (cp = retptr; *cp; cp++)
          if (*cp == '\\')
              *cp = '/';

--- 47,64 ----
          elog(FATAL, "could not get size for full pathname of datadir %s: %lu",
               DataDir, GetLastError());

!     retptr = malloc(bufsize  + 18);        /* 1 NULL and 18 for
                                           * Global\PostgreSQL: */
      if (retptr == NULL)
          elog(FATAL, "could not allocate memory for shared memory name");

      strcpy(retptr, "Global\\PostgreSQL:");
!     r = GetFullPathName(DataDir, bufsize, retptr + 18, NULL);
      if (r == 0 || r > bufsize)
          elog(FATAL, "could not generate full pathname for datadir %s: %lu",
               DataDir, GetLastError());

!     for (cp = retptr + 18; *cp; cp++)
          if (*cp == '\\')
              *cp = '/';