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, May 29, 2015 at 08:36:23PM +0000, Joseph Myers wrote:
> 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.
>
Possible but I don't know how to do that yet. A problem I try avoid are
interdependencies. Without making that optional with ifdef to make it
optional you would need include other file. It could result on having
five phases of this header and you need to add redefinition between
correct headers to work. 
 
> > +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.
> 
will do.
> > +#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.
> 
Will add MIN_PAGE_SIZE in next version.
> > +#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.
> 
These are for strn* functions to check a if we read past input. My
original implementation of these checks in memchr was overkill so I will
simplify these checks.


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