incubator-lucy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Marvin Humphrey <>
Subject Re: [Lucy] Re: quiet gcc warnings
Date Wed, 09 Dec 2009 05:24:44 GMT
On Tue, Dec 08, 2009 at 08:33:45PM -0600, Peter Karman wrote:
> >My approach is to avoid ptrdiff_t altogether and always upconvert all 
> >operands
> >to i64_t before performing the arithmetic.  For example[1]:
> >
> >    static INLINE i64_t
> >    SI_tell(InStream *self)
> >    {
> >        FileWindow *const window = self->window;
> >        i64_t pos_in_buf = PTR2I64(self->buf) - PTR2I64(window->buf);
> >        return pos_in_buf + window->offset - self->offset;
> >    }
> >
> yes, makes sense.

I should mention for the record that technically it's a violation of the C
standard to perform arbitrary pointer math.  This is officially undefined:

  char *foo = (char*)malloc(2);
  foo = foo--;

The only officially valid addresses with regards to that pointer are:

  foo + 1
  foo + 2


However, on common modern systems, you get away with it... and in fact
InStream's design depends on that construct working.

> >A thought: should we only define PTR2I32 and PTR2U32 on 32-bit systems?
> >They'd silently discard data on 64-bit systems.  That would be bad.  
> >
> >We should force the programmer to either wrap the PTR2x32 conversion in an
> >#ifdef, or go through PTR2x64 first and then truncate with an explicit cast
> >which verifies their intent.
> >
> >    /* Assume that "start" and "end" cannot be more than I32_MAX apart. */
> >    #if (SIZEOF_PTR == 4)
> >       i32_t length = PTR2I32(end) - PTR2I32(start);
> >    #else
> >       i64_t a = PTR2I64(start);
> >       i64_t b = PTR2I64(end);
> >       i32_t length = (i32_t)(b - a);
> >    #endif
> >
> yes, that seems sane.
> Shall I have a go at this for my next task, or do you have something else 
> in mind?

That would be a good one.

Anything that moves us towards the goal of an easy to understand and use
Charmonizer code base would be handy.  

The next step is probably for me to redraft the top level docs -- Charmonizer
has the functionality it needs, but the organization can be improved.  Then if
I can persuade you and Nate to critique them, we'll refactor accordingly.

Since Charmonizer is an internal API, it doesn't have to be perfect, but it's
a dialect of C that Lucy contributors will have to familiarize themselves with
and the less effort that takes the better.


Along those lines, it would be nice if we could phase out all the
Charmonizer-specific integer types and replace them with types from the C99
header stdint.h.

So, instead of "u32_t" or "chy_u32_t", we'd use "uint32_t" everywhere.

We could actually generate a complete and usable stdint.h on systems where it
isn't available using no more compilation tests than we already have going in
Charmonizer/Probe/Integers.c.  But as a quick and dirty fix, we can either
pound-include stdint.h in charmony.h if it's there, or change the naming of
our typedefs if it's not.

Marvin Humphrey

View raw message