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 Mon, 30 Nov 2009 00:36:37 GMT
On Sat, Nov 28, 2009 at 08:58:04PM -0600, Peter Karman wrote:

> > There's already a PTR2I64 macro in Charmonizer/Probe/Integers, but we probably
> > need a larger menu:
> > 
> >    PTR2I64  
> >    PTR2U64  
> >    PTR2I32 
> >    PTR2U32 
> >    PTR2SIZET
> sounds reasonable.
> Should pointers always be converted to unsigned types?

Sometimes if you're subtracting two pointers from each other, you need the
difference to be a signed result.  The type "ptrdiff_t" exists for this
reason, but it's tricky to use correctly because you have to think about how
it gets upconverted on different systems.

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;

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);
       i64_t a = PTR2I64(start);
       i64_t b = PTR2I64(end);
       i32_t length = (i32_t)(b - a);

[1] The instream->window->buf pointer is used to keep track of where the
    mmap'd region starts, and instream->buf serves as the cursor.  To tell
    where we are in the file, we need to do some pointer math, then add the
    result into an i64_t file position calculation.  We don't have to worry
    about integer promotion in the last line, because all three operands have
    the type i64_t.

Marvin Humphrey

View raw message