This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v4] generic string skeleton.


On Fri, 29 May 2015, Ondřej Bílka wrote:

> +#ifndef VECTOR_INT
> +# define VECTOR_INT unsigned long int
> +#endif

I think a separate header for this would be better to avoid the #ifndef 
pattern.  I also wonder if actually this information about register size 
(which is effectively what this is) really ought to go in bits/wordsize.h 
in some way, with other headers then working from that.  Because it's not 
just this code that can use such information - gmp-mparam.h can (it ought 
to be possible to eliminate machine-specific versions of gmp-mparam.h) as 
can sfp-machine.h (quite a bit of the sfp-machine.h files is actually 
generic).  But since bits/wordsize.h is installed, there's a case for this 
going in a non-installed header, where the default version just uses 
bits/wordsize.h.

> +static const vector_int ones = (~0UL / 255); /* 0x0101...*/
> +static const vector_int add = 127 * (~0UL / 255);
> +static const vector_int high_bits = 128 * (~0UL / 255);

These need to use ((vector_int) -1) or similar instead of ~0UL, for when 
the type is wider than int.

> +#define LSIZE sizeof (vector_int)
> +#ifdef PAGE_SIZE
> +# define CROSS_PAGE(x, n) (((uintptr_t) x) % PAGE_SIZE > PAGE_SIZE - n)

If PAGE_SIZE might sometimes be nonconstant (see sysdeps/mach/pagecopy.h), 
I'd tend to think a separate macro (for the minimum size of a page, always 
constant) would be better here.

> +#ifndef BOUND
> +# define BOUND(x) 0
> +#endif

I've no idea what the semantics of this macro are.  It definitely needs a 
comment.  Similarly for subsequent macros in this header.

-- 
Joseph S. Myers
joseph@codesourcery.com

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]