> -----Original Message-----
> From: Ben Pfaff [mailto:[email protected]]
> Sent: Wednesday, April 19, 2017 7:46 AM
> To: Alin Serdean <[email protected]>
> Cc: [email protected]
> Subject: Re: [ovs-dev] [PATCH 03/10] windows: add includes to daemon-
> windows
> 
> On Wed, Apr 19, 2017 at 03:07:03AM +0000, Alin Serdean wrote:
> > > > -    fprintf(filep_pidfile, "%d\n", _getpid());
> > > > +    fprintf(filep_pidfile, "%d\n", getpid());
> > >
> > > This seems reasonable to me, except that usual practice would be
> > > more like
> > > this:
> > >     fprintf(filep_pidfile, "%ld\n", (long int) getpid()); because
> > > pid_t might be short or int or long.
> > [Alin Serdean] Thanks for the review Ben, I totally missed that.
> > Although being MSFT specific if we replace getpid with
> GetCurrentProcessId (https://msdn.microsoft.com/en-
> us/library/windows/desktop/ms683180(v=vs.85).aspx) and from includes:
> > typedef unsigned long       DWORD;
> > I'm wondering if we should switch it to `%lu` and `(unsigned long)`.
> > What do you think?
> 
> Hmm.
> 
> POSIX says that pid_t must be a signed integer type.  An unsigned integer
> type for pid_t breaks things; for example, the POSIX wait() function has a
> pid_t return type and returns -1 to indicate an error.
> So it's somewhat distressing to have getpid() return an unsigned long, since
> that breaks real programs' fairly justified assumptions that
> getpid() returns a signed integer type.
> 
> What if, instead of "#define getpid() _getpid()", we used an inline function,
> e.g.
> 
>     static inline pid_t getpid(void)
>     {
>         return GetCurrentProcessId();
>     }
> 
> That would be cleaner from a POSIX point of view.
> 
> (On the other hand, if GetCurrentProcessId() might return a DWORD so big
> that it becomes a negative pid_t, we need to have a bigger discussion.)
[Alin Serdean] I agree it would look cleaner from a POSIX point of view, but I 
need to expand both functions to see if they call the same thing behind the 
scenes, so we can be sure it won't rollover. 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to