This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v4] generic string skeleton.
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: Joseph Myers <joseph at codesourcery dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Sat, 30 May 2015 22:23:56 +0200
- Subject: Re: [PATCH v4] generic string skeleton.
- Authentication-results: sourceware.org; auth=none
- References: <20150529190952 dot GA23952 at domone> <alpine dot DEB dot 2 dot 10 dot 1505292028370 dot 17156 at digraph dot polyomino dot org dot uk>
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.