[prev in list] [next in list] [prev in thread] [next in thread] 

List:       ast-developers
Subject:    [ast-developers] Re: [patch] Patch to replace some |open()|+|fcntl(..., F_CLOEXEC)|  with |open(...,
From:       Glenn Fowler <gsf () research ! att ! com>
Date:       2012-06-18 20:20:00
Message-ID: 201206182020.q5IKK0HW006769 () terra ! research ! att ! com
[Download RAW message or body]


On Mon, 18 Jun 2012 21:54:27 +0200 Roland Mainz wrote:
> On Mon, Jun 18, 2012 at 9:05 PM, Glenn Fowler <gsf@research.att.com> wrote:
> >> Attached (as "astopen20120612_o_cloexec_patch20120616_001.diff") is a
> >> patch which replaces some of the |open()|+|fcntl(...,F_CLOEXEC)|
> >> sequences in ast-open.2012-06-12 with  |open(...,O_CLOEXEC)|  if the
> >> |O_CLOEXEC| CPP symbol is available on that platform (I've tested with
> >> Solaris 11 and SuSE 12.1).
> >
> >> IMO the patch is simple&&safe enough for ksh93u+, too.
> >
> > thanks

> No problem... the main idea behind this was to reduce the size of code
> a bit and get minor performance improvements (e.g. fewer number of
> syscalls).

> > dgk and I coordinated on your suggestion
> > the iffe-generated <ast_fcntl.h> will do this if O_CLOEXEC is not defined
> >        #define O_CLOEXEC 0
> > just like we did with { O_BINARY O_TEXT }
> > this will cut down on #ifdefs

> Uhhh... is this wise ? How do you test whether |O_CLOEXEC| is really
> supported by a platform or not ? Wouldn't it be "safer" to define
> something like |AST_O_CLOEXEC| (see also follow-up email about
> |fcntl(...,*CLOEXEC*,...|&co.) instead ? IMO asking because 3rd-party
> code may use <stdio.h> from libast... wouldn't it be bitten in some
> form by this if it does a #ifdef O_CLOEXEC ... #else ... #endif
> without being aware that libast headers already did the |#define
> O_CLOEXEC 0| ?

right
comparing O_CLOEXEC to { O_TEXT O_BINARY } is not valid

_______________________________________________
ast-developers mailing list
ast-developers@research.att.com
https://mailman.research.att.com/mailman/listinfo/ast-developers
[prev in list] [next in list] [prev in thread] [next in thread]