Thread

Re: pgsql: Remove "fmgr.h" include in cube contrib --- caused crash on a Ge

Bruce Momjian <bruce@momjian.us> — 2011-09-02T16:44:28Z

Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > I wonder if this would be the right time to start using the
> > FLEXIBLE_ARRAY_MEMBER stuff in contrib/cube.  Note pg_config.h.in says
> 
> > /* Define to nothing if C supports flexible array members, and to 1 if it does
> >    not. That way, with a declaration like `struct s { int n; double
> >    d[FLEXIBLE_ARRAY_MEMBER]; };', the struct hack can be used with pre-C99
> >    compilers. When computing the size of such an object, don't use 'sizeof
> >    (struct s)' as it overestimates the size. Use 'offsetof (struct s, d)'
> >    instead. Don't use 'offsetof (struct s, d[0])', as this doesn't work with
> >    MSVC and with C++ compilers. */
> 
> D'oh ... I bet that last sentence is pointing us at the problem.  cube
> is using exactly that construct, and for some reason it's crashing.
> The most likely explanation for why it's crashing is that the compiler
> is trying to dereference NULL instead of successfully reducing
> offsetof() to a compile-time constant.  It's still not too clear to me
> why the inclusion changes cause that, but certainly walsender.h is
> pulling in a crapload of other stuff that was not previously included
> there ... which connects back to my previous complaints that I think
> Bruce was way too aggressive in adding #includes to headers.
> 
> Jeremy, could you look at the preprocessor output for cube.c (ie,
> use -E instead of -c in the gcc call) and see how the relevant line
> of cube_f8_f8 looks in both broken and non-broken cases?  What I see
> on a Fedora box is
> 
>  size = __builtin_offsetof (NDBOX, x[0]) +sizeof(double) * 2;
> 
> but I'm thinking you might be getting something different.

I see 35 instances of this coding, and only 12 are in contrib/cube; 
examples attached.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + It's impossible for everything to be true. +