Horton

Thread

  1. xlog.c.patch for cygwin port.

    Alexei Zakharov <a.s.zakharov@inp.nsk.su> — 2000-03-06T06:06:27Z

    Hello,

I've recently written to pgsql-ports about a problem with PG7.0 on NT (Subj: [PORTS] initdb problem on NT with 7.0).  Since nobody helped me, I had to find out the reson.  The difference between NT and Linux (for instance) is that "open( path, O_RDWR );" opens a file in text mode.  So sometime less block can be read than required.

I suggest a following patch.  BTW the situation appeared before, see hba.c, pqcomm.c and others.

  2. Re: [HACKERS] xlog.c.patch for cygwin port.

    Yutaka tanida <yutaka@marin.or.jp> — 2000-03-07T05:25:22Z

    Hi,

On Mon, 6 Mar 2000 12:06:27 +0600
"Alexei Zakharov" <A.S.Zakharov@inp.nsk.su> wrote:

> I suggest a following patch.  BTW the situation appeared before, 
> see hba.c, pqcomm.c and others.

I propose another patch, against src/include/port/win.h.
If this patch is applied, #ifdef with open() such as following is no
more needed.

--
#ifndef __CYGWIN__
  fd=open(path,flags,mode);
#else
  fd=open(path,flags | O_BINARY,mode);
#endif
--

Comments?

--
Yutaka tanida / S34 Co., Ltd.
tanida@s34.co.jp (Office)
yutaka@marin.or.jp(Private, or if you *HATE* Microsoft Outlook)



*** win.h.orig	Thu Feb 10 17:00:14 2000
--- win.h	Tue Mar 07 14:07:21 2000
***************
*** 10,15 ****
--- 10,35 ----
  #define USE_POSIX_TIME
  #define HAVE_INT_TIMEZONE		/* has int _timezone */
  
+ /* open() must use with O_BINARY flag */
+ 
+ #include<stdarg.h>
+ #include<sys/fcntl.h>
+ 
+ static __inline int pg_cygwin_open(const char *pathname,int flags,...) {
+     va_list va;
+ 	mode_t mode;
+ 	if(flags | O_CREAT) {
+         va_start(va,flags);
+         mode=va_arg(va,int);
+         va_end(va);
+     	return open(pathname,flags | O_BINARY,mode);
+ 	}else{
+     	return open(pathname,flags | O_BINARY);
+ 	}
+ }
+ 
+ #define open pg_cygwin_open
+ 
  #include <cygwin/version.h>
  #if (CYGWIN_VERSION_API_MAJOR >= 0) && (CYGWIN_VERSION_API_MINOR >= 8)
  #define sys_nerr _sys_nerr

  3. Re: [HACKERS] xlog.c.patch for cygwin port.

    Bruce Momjian <pgman@candle.pha.pa.us> — 2000-03-07T23:48:55Z

    Applied.

[Charset Windows-1252 unsupported, skipping...]

[Attachment, skipping...]


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

  4. Re: [HACKERS] xlog.c.patch for cygwin port.

    Bruce Momjian <pgman@candle.pha.pa.us> — 2000-03-08T00:13:06Z

    This looks interesting.  We could remove some of our ifwin cruft.

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


On Mon, 6 Mar 2000 12:06:27 +0600
"Alexei Zakharov" <A.S.Zakharov@inp.nsk.su> wrote:

> I suggest a following patch.  BTW the situation appeared before, 
> see hba.c, pqcomm.c and others.

I propose another patch, against src/include/port/win.h.
If this patch is applied, #ifdef with open() such as following is no
more needed.

--
#ifndef __CYGWIN__
  fd=open(path,flags,mode);
#else
  fd=open(path,flags | O_BINARY,mode);
#endif
--

Comments?

--
Yutaka tanida / S34 Co., Ltd.
tanida@s34.co.jp (Office)
yutaka@marin.or.jp(Private, or if you *HATE* Microsoft Outlook)



*** win.h.orig	Thu Feb 10 17:00:14 2000
--- win.h	Tue Mar 07 14:07:21 2000
***************
*** 10,15 ****
--- 10,35 ----
  #define USE_POSIX_TIME
  #define HAVE_INT_TIMEZONE		/* has int _timezone */
  
+ /* open() must use with O_BINARY flag */
+ 
+ #include<stdarg.h>
+ #include<sys/fcntl.h>
+ 
+ static __inline int pg_cygwin_open(const char *pathname,int flags,...) {
+     va_list va;
+ 	mode_t mode;
+ 	if(flags | O_CREAT) {
+         va_start(va,flags);
+         mode=va_arg(va,int);
+         va_end(va);
+     	return open(pathname,flags | O_BINARY,mode);
+ 	}else{
+     	return open(pathname,flags | O_BINARY);
+ 	}
+ }
+ 
+ #define open pg_cygwin_open
+ 
  #include <cygwin/version.h>
  #if (CYGWIN_VERSION_API_MAJOR >= 0) && (CYGWIN_VERSION_API_MINOR >= 8)
  #define sys_nerr _sys_nerr


************


-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

  5. Re: [HACKERS] xlog.c.patch for cygwin port.

    Tom Lane <tgl@sss.pgh.pa.us> — 2000-03-08T01:01:16Z

    Bruce Momjian <pgman@candle.pha.pa.us> writes:
> This looks interesting.  We could remove some of our ifwin cruft.

I have been thinking for quite some time that most of the CYGWIN32
ifdefs represent very poor programming.  Instead of zillions of

#ifndef __CYGWIN32__
	fd = open(filename, O_RDONLY, 0666);
#else
	fd = open(filename, O_RDONLY | O_BINARY, 0666);
#endif

we should have in one include file something like

#ifndef __CYGWIN32__
#define  OPEN_FLAGS_READ   O_RDONLY
#define  OPEN_FLAGS_WRITE  O_WRONLY
// etc for the combinations we need
#else
#define  OPEN_FLAGS_READ   (O_RDONLY | O_BINARY)
#define  OPEN_FLAGS_WRITE  (O_WRONLY | O_BINARY)
// etc
#endif

and then the body of the code would have

	fd = open(filename, OPEN_FLAGS_READ, 0666);

and no ifdef.  This would also provide a single place to tweak open()
flags for other platforms, whereas the existing method is exactly zero
help for any non-CYGWIN platform that wants to add O_BINARY ...

			regards, tom lane

  6. Re: [HACKERS] xlog.c.patch for cygwin port.

    Marc G. Fournier <scrappy@hub.org> — 2000-03-08T01:19:09Z

    Sounds like a *great* bug fix to me ... if you "have better things to do",
I can tackle it ...

On Tue, 7 Mar 2000, Tom Lane wrote:

> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > This looks interesting.  We could remove some of our ifwin cruft.
> 
> I have been thinking for quite some time that most of the CYGWIN32
> ifdefs represent very poor programming.  Instead of zillions of
> 
> #ifndef __CYGWIN32__
> 	fd = open(filename, O_RDONLY, 0666);
> #else
> 	fd = open(filename, O_RDONLY | O_BINARY, 0666);
> #endif
> 
> we should have in one include file something like
> 
> #ifndef __CYGWIN32__
> #define  OPEN_FLAGS_READ   O_RDONLY
> #define  OPEN_FLAGS_WRITE  O_WRONLY
> // etc for the combinations we need
> #else
> #define  OPEN_FLAGS_READ   (O_RDONLY | O_BINARY)
> #define  OPEN_FLAGS_WRITE  (O_WRONLY | O_BINARY)
> // etc
> #endif
> 
> and then the body of the code would have
> 
> 	fd = open(filename, OPEN_FLAGS_READ, 0666);
> 
> and no ifdef.  This would also provide a single place to tweak open()
> flags for other platforms, whereas the existing method is exactly zero
> help for any non-CYGWIN platform that wants to add O_BINARY ...
> 
> 			regards, tom lane
> 
> ************
> 

Marc G. Fournier                   ICQ#7615664               IRC Nick: Scrappy
Systems Administrator @ hub.org 
primary: scrappy@hub.org           secondary: scrappy@{freebsd|postgresql}.org

  7. Re: [HACKERS] xlog.c.patch for cygwin port.

    Bruce Momjian <pgman@candle.pha.pa.us> — 2000-03-08T01:50:07Z

    > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > This looks interesting.  We could remove some of our ifwin cruft.
> 
> I have been thinking for quite some time that most of the CYGWIN32
> ifdefs represent very poor programming.  Instead of zillions of
> 
> #ifndef __CYGWIN32__
> 	fd = open(filename, O_RDONLY, 0666);
> #else
> 	fd = open(filename, O_RDONLY | O_BINARY, 0666);
> #endif
> 
> we should have in one include file something like

Do we ever assign a function pointer for open() anywhere.  If so, the
define will not work without some kind of wrapper, right?

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

  8. Re: [HACKERS] xlog.c.patch for cygwin port.

    D. Jay Newman <jay@sprucegrove.com> — 2000-03-08T02:48:28Z

    >> Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> > This looks interesting.  We could remove some of our ifwin cruft.
>> 
>> I have been thinking for quite some time that most of the CYGWIN32
>> ifdefs represent very poor programming.  Instead of zillions of
>> 
>> #ifndef __CYGWIN32__
>> 	fd = open(filename, O_RDONLY, 0666);
>> #else
>> 	fd = open(filename, O_RDONLY | O_BINARY, 0666);
>> #endif
>> 
>> we should have in one include file something like
>
>Do we ever assign a function pointer for open() anywhere.  If so, the
>define will not work without some kind of wrapper, right?

Since the only difference seems to be "O_RDONLY" vs "O_RDONLY | O_BINARY",
why not do the #define on that?

At least in this case it works.
-- 
D. Jay Newman                   ! For the pleasure and the profit it derives
jay@sprucegrove.com              ! I arrange things, like furniture, and
http://www.sprucegrove.com/~jay/   ! daffodils, and ...lives.  -- Hello Dolly

  9. Re: [HACKERS] xlog.c.patch for cygwin port.

    Tatsuo Ishii <t-ishii@sra.co.jp> — 2000-03-08T03:03:11Z

    > >> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> >> > This looks interesting.  We could remove some of our ifwin cruft.
> >> 
> >> I have been thinking for quite some time that most of the CYGWIN32
> >> ifdefs represent very poor programming.  Instead of zillions of
> >> 
> >> #ifndef __CYGWIN32__
> >> 	fd = open(filename, O_RDONLY, 0666);
> >> #else
> >> 	fd = open(filename, O_RDONLY | O_BINARY, 0666);
> >> #endif
> >> 
> >> we should have in one include file something like
> >
> >Do we ever assign a function pointer for open() anywhere.  If so, the
> >define will not work without some kind of wrapper, right?
> 
> Since the only difference seems to be "O_RDONLY" vs "O_RDONLY | O_BINARY",
> why not do the #define on that?
> 
> At least in this case it works.

BTW, why do we call open() directory here? Why not VFD interface?
--
Tatsuo Ishii

  10. Re: [HACKERS] xlog.c.patch for cygwin port.

    Marc G. Fournier <scrappy@hub.org> — 2000-03-08T05:42:06Z

    On Tue, 7 Mar 2000, Bruce Momjian wrote:

> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > This looks interesting.  We could remove some of our ifwin cruft.
> > 
> > I have been thinking for quite some time that most of the CYGWIN32
> > ifdefs represent very poor programming.  Instead of zillions of
> > 
> > #ifndef __CYGWIN32__
> > 	fd = open(filename, O_RDONLY, 0666);
> > #else
> > 	fd = open(filename, O_RDONLY | O_BINARY, 0666);
> > #endif
> > 
> > we should have in one include file something like
> 
> Do we ever assign a function pointer for open() anywhere.  If so, the
> define will not work without some kind of wrapper, right?

Okay, I'm lost ... if we "#define OPEN_FLAGS .." and not the open itself,
why would we need some kind of wrapper?

  11. Re: [HACKERS] xlog.c.patch for cygwin port.

    Bruce Momjian <pgman@candle.pha.pa.us> — 2000-03-08T05:50:15Z

    > On Tue, 7 Mar 2000, Bruce Momjian wrote:
> 
> > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > > This looks interesting.  We could remove some of our ifwin cruft.
> > > 
> > > I have been thinking for quite some time that most of the CYGWIN32
> > > ifdefs represent very poor programming.  Instead of zillions of
> > > 
> > > #ifndef __CYGWIN32__
> > > 	fd = open(filename, O_RDONLY, 0666);
> > > #else
> > > 	fd = open(filename, O_RDONLY | O_BINARY, 0666);
> > > #endif
> > > 
> > > we should have in one include file something like
> > 
> > Do we ever assign a function pointer for open() anywhere.  If so, the
> > define will not work without some kind of wrapper, right?
> 
> Okay, I'm lost ... if we "#define OPEN_FLAGS .." and not the open itself,
> why would we need some kind of wrapper?

No, the original person was refining open().  Ithink defining the flags
is much better.

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

  12. Re: [HACKERS] xlog.c.patch for cygwin port.

    Marc G. Fournier <scrappy@hub.org> — 2000-03-08T06:07:28Z

    On Wed, 8 Mar 2000, Bruce Momjian wrote:

> > On Tue, 7 Mar 2000, Bruce Momjian wrote:
> > 
> > > > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > > > This looks interesting.  We could remove some of our ifwin cruft.
> > > > 
> > > > I have been thinking for quite some time that most of the CYGWIN32
> > > > ifdefs represent very poor programming.  Instead of zillions of
> > > > 
> > > > #ifndef __CYGWIN32__
> > > > 	fd = open(filename, O_RDONLY, 0666);
> > > > #else
> > > > 	fd = open(filename, O_RDONLY | O_BINARY, 0666);
> > > > #endif
> > > > 
> > > > we should have in one include file something like
> > > 
> > > Do we ever assign a function pointer for open() anywhere.  If so, the
> > > define will not work without some kind of wrapper, right?
> > 
> > Okay, I'm lost ... if we "#define OPEN_FLAGS .." and not the open itself,
> > why would we need some kind of wrapper?
> 
> No, the original person was refining open().  Ithink defining the flags
> is much better.

	Ah, okay, knew I was missing something :)

  13. Re: xlog.c.patch for cygwin port.

    Bruce Momjian <pgman@candle.pha.pa.us> — 2000-06-09T16:03:17Z

    We have resolved all the open()/binary flag calls.



-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

  14. Re: xlog.c.patch for cygwin port.

    Bruce Momjian <pgman@candle.pha.pa.us> — 2000-06-09T16:04:10Z

    We have resolved all the O_BINARY calls for 7.1.  Thanks.


> On Mon, 6 Mar 2000 12:06:27 +0600
> "Alexei Zakharov" <A.S.Zakharov@inp.nsk.su> wrote:
> 
> > I suggest a following patch.  BTW the situation appeared before, 
> > see hba.c, pqcomm.c and others.
> 
> I propose another patch, against src/include/port/win.h.
> If this patch is applied, #ifdef with open() such as following is no
> more needed.
> 
> --
> #ifndef __CYGWIN__
>   fd=open(path,flags,mode);
> #else
>   fd=open(path,flags | O_BINARY,mode);
> #endif

-- 
  Bruce Momjian                        |  http://www.op.net/~candle
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026