Re: Update nwlib/libc/search directory

Hi

It looks like a lot of the changes are to move to standard fixed width
types like uint32_t and clean up. I see the calloc lost a cast and some #if
conditionals lost parentheses.

There is an ifdef for 64-bit stat(). I don't know if newlib has a
conditional that should be used to trip that feature or not. But letting it
default to 32-bit stat would be OK to get it working and then we all can
decide how/if to trip that based on something in newlib.

I noticed a change to use pwrite() instead of write() but that should be
OK. It is in <unistd.h> per POSIX.

If the question is to use the new version and continue pushing, then the
answer is yes.

--joel

On Wed, Jul 3, 2019 at 3:13 AM Vaibhav Gupta <[email protected]>
wrote:

>
>
> On Tue, Jul 2, 2019 at 8:58 PM Joel Sherrill <[email protected]> wrote:
>
>>
>>
>> On Tue, Jul 2, 2019 at 6:41 AM Vaibhav Gupta <[email protected]>
>> wrote:
>>
>>> Hello everyone,
>>> I have ported    ndbm.h    to newlib from FreeBSD.
>>> I have placed     ndbm.h     to     newlib/libc/include     directory
>>> and
>>> ndbm.c     to    newlib/libc/posix     directory .
>>>
>>> But I realized, the function declaration of __hash_open in newlib is:
>>> DB      *__hash_open(const char *, int, int, int, const HASHINFO *);
>>>
>>> and in FreeBSD it is:
>>> DB      *__hash_open(const char *, int, int, const HASHINFO *, int);
>>> .
>>> .
>>> .
>>> .
>>> Although I made required changes in    ndbm.c      but I guess maybe the
>>> entire search.h should be updated to newer FreeBSD resource?
>>> Should we do that?
>>>
>>
>> I think so. It looks like previous merges have cherry picked code needed
>> for
>> ndbm support and it all needs to be updated in sync.
>>
> Brian replied to my mail last time. He pointed out various updates:
> https://sourceware.org/ml/newlib/2019/msg00250.html
>
>>
>> Out of curiosity, are the parameters all the same but the last two are in
>> different
>> order or did something more substantial change in the __hash_open
>> signature?
>>
> at few places I saw introduction of new functions, may be they are for
> optimizations.
> newlib -
> https://github.com/VARoDeK/newlib/blob/master/newlib/libc/search/hash.c
> freebsd -
> https://github.com/freebsd/freebsd/blob/master/lib/libc/db/hash/hash.c
> =======================================
> Here is the output generated by 'diff' command;
>
> $ ~/temp  diff hashnewlib.c hashfreeBSD.c
> 1a2,3
> >  * SPDX-License-Identifier: BSD-3-Clause
> >  *
> 16,20c18
> <  * 3. All advertising materials mentioning features or use of this
> software
> <  *    must display the following acknowledgement:
> <  * This product includes software developed by the University of
> <  * California, Berkeley and its contributors.
> <  * 4. Neither the name of the University nor the names of its
> contributors
> ---
> >  * 3. Neither the name of the University nor the names of its
> contributors
> 37d34
> < #include <sys/param.h>
> 42c39
> < #include <sys/types.h>
> ---
> > __FBSDID("$FreeBSD$");
> 43a41,42
> > #include "namespace.h"
> > #include <sys/param.h>
> 54a54
> > #include "un-namespace.h"
> 56,57c56
> < #define __DBINTERFACE_PRIVATE /* activate prototypes from db_local.h */
> < #include "db_local.h"
> ---
> > #include <db.h>
> 66c65
> < static int   hash_delete(const DB *, const DBT *, u_int);
> ---
> > static int   hash_delete(const DB *, const DBT *, u_int32_t);
> 68,69c67,68
> < static int   hash_get(const DB *, const DBT *, DBT *, u_int);
> < static int   hash_put(const DB *, DBT *, const DBT *, u_int);
> ---
> > static int   hash_get(const DB *, const DBT *, DBT *, u_int32_t);
> > static int   hash_put(const DB *, DBT *, const DBT *, u_int32_t);
> 71,72c70,71
> < static int   hash_seq(const DB *, DBT *, DBT *, u_int);
> < static int   hash_sync(const DB *, u_int);
> ---
> > static int   hash_seq(const DB *, DBT *, DBT *, u_int32_t);
> > static int   hash_sync(const DB *, u_int32_t);
> 76c75
> < #if (BYTE_ORDER == LITTLE_ENDIAN)
> ---
> > #if BYTE_ORDER == LITTLE_ENDIAN
> 81,88d79
> < /* Macros for min/max.  */
> < #ifndef MIN
> < #define MIN(a,b) (((a)<(b))?(a):(b))
> < #endif
> < #ifndef MAX
> < #define MAX(a,b) (((a)>(b))?(a):(b))
> < #endif
> <
> 106,111c97,101
> < extern DB *
> < __hash_open (const char *file,
> < int flags,
> < int mode,
> < int dflags,
> < const HASHINFO *info) /* Special directives for create */
> ---
> > /* ARGSUSED */
> > DB *
> > __hash_open(const char *file, int flags, int mode,
> >     const HASHINFO *info, /* Special directives for create */
> >     int dflags)
> 114,117d103
> <
> < #ifdef __USE_INTERNAL_STAT64
> <         struct stat64 statbuf;
> < #else
> 119d104
> < #endif
> 140,150d124
> < new_table = 0;
> < if (!file || (flags & O_TRUNC) ||
> < #ifdef __USE_INTERNAL_STAT64
> <    (_stat64(file, &statbuf) && (errno == ENOENT))) {
> < #else
> <    (stat(file, &statbuf) && (errno == ENOENT))) {
> < #endif
> < if (errno == ENOENT)
> < errno = 0; /* Just in case someone looks at errno */
> < new_table = 1;
> < }
> 152c126
> < if ((hashp->fp = open(file, flags, mode)) == -1)
> ---
> > if ((hashp->fp = _open(file, flags | O_CLOEXEC, mode)) == -1)
> 153a128,131
> > new_table = _fstat(hashp->fp, &statbuf) == 0 &&
> >    statbuf.st_size == 0 && (flags & O_ACCMODE) != O_RDONLY;
> > } else
> > new_table = 1;
> 155,168d132
> < /* if the .db file is empty, and we had permission to create
> <   a new .db file, then reinitialize the database */
> < if ((flags & O_CREAT) &&
> < #ifdef __USE_INTERNAL_STAT64
> <     _fstat64(hashp->fp, &statbuf) == 0 && statbuf.st_size == 0)
> < #else
> <     fstat(hashp->fp, &statbuf) == 0 && statbuf.st_size == 0)
> < #endif
> < new_table = 1;
> <
> < #ifdef HAVE_FCNTL
> < (void)fcntl(hashp->fp, F_SETFD, 1);
> < #endif
> < }
> 170c134
> < if (!(hashp = init_hash(hashp, file, (HASHINFO *)info)))
> ---
> > if (!(hashp = init_hash(hashp, file, info)))
> 179,180c143,144
> < hdrsize = read(hashp->fp, &hashp->hdr, sizeof(HASHHDR));
> < #if (BYTE_ORDER == LITTLE_ENDIAN)
> ---
> > hdrsize = _read(hashp->fp, &hashp->hdr, sizeof(HASHHDR));
> > #if BYTE_ORDER == LITTLE_ENDIAN
> 191,192c155,156
> < if (hashp->HASH_VERSION != HASHVERSION &&
> <    hashp->HASH_VERSION != OLDHASHVERSION)
> ---
> > if (hashp->VERSION != HASHVERSION &&
> >    hashp->VERSION != OLDHASHVERSION)
> 194c158
> < if (hashp->hash(CHARKEY, sizeof(CHARKEY)) != hashp->H_CHARKEY)
> ---
> > if ((int32_t)hashp->hash(CHARKEY, sizeof(CHARKEY)) != hashp->H_CHARKEY)
> 196,198d159
> <                 /* Check bucket size isn't too big for target int. */
> <                 if (hashp->BSIZE > INT_MAX)
> <                         RETURN_ERROR(EFTYPE, error1);
> 204,206c165
> < nsegs = (hashp->MAX_BUCKET + 1 + hashp->SGSIZE - 1) /
> < hashp->SGSIZE;
> < hashp->nsegs = 0;
> ---
> > nsegs = howmany(hashp->MAX_BUCKET + 1, hashp->SGSIZE);
> 219c178
> < (void)memset(&hashp->mapp[0], 0, bpages * sizeof(__uint32_t *));
> ---
> > (void)memset(&hashp->mapp[0], 0, bpages * sizeof(u_int32_t *));
> 249c208
> <
> "%s\n%s%x\n%s%d\n%s%d\n%s%d\n%s%d\n%s%d\n%s%d\n%s%d\n%s%d\n%s%d\n%s%x\n%s%x\n%s%d\n%s%d\n",
> ---
> >
> "%s\n%s%p\n%s%d\n%s%d\n%s%d\n%s%d\n%s%d\n%s%d\n%s%d\n%s%d\n%s%d\n%s%x\n%s%x\n%s%d\n%s%d\n",
> 273c232
> < (void)close(hashp->fp);
> ---
> > (void)_close(hashp->fp);
> 282,283c241
> < hash_close(dbp)
> < DB *dbp;
> ---
> > hash_close(DB *dbp)
> 298,299c256
> < hash_fd(dbp)
> < const DB *dbp;
> ---
> > hash_fd(const DB *dbp)
> 316,319c273
> < init_hash(hashp, file, info)
> < HTAB *hashp;
> < const char *file;
> < const HASHINFO *info;
> ---
> > init_hash(HTAB *hashp, const char *file, const HASHINFO *info)
> 321,323d274
> < #ifdef __USE_INTERNAL_STAT64
> <         struct stat64 statbuf;
> < #else
> 325d275
> < #endif
> 330c280
> <        hashp->LORDER = DB_BYTE_ORDER;
> ---
> > hashp->LORDER = BYTE_ORDER;
> 343,345d292
> < #ifdef __USE_INTERNAL_STAT64
> < if (_stat64(file, &statbuf))
> < #else
> 347d293
> < #endif
> 349c295,297
> < hashp->BSIZE = MIN(statbuf.st_blksize, MAX_BSIZE);
> ---
> > hashp->BSIZE = statbuf.st_blksize;
> > if (hashp->BSIZE > MAX_BSIZE)
> > hashp->BSIZE = MAX_BSIZE;
> 370,371c318,319
> <                        if (info->lorder != DB_BIG_ENDIAN &&
> <                            info->lorder != DB_LITTLE_ENDIAN) {
> ---
> > if (info->lorder != BIG_ENDIAN &&
> >    info->lorder != LITTLE_ENDIAN) {
> 391,393c339
> < init_htab(hashp, nelem)
> < HTAB *hashp;
> < int nelem;
> ---
> > init_htab(HTAB *hashp, int nelem)
> 395,396c341
> < int nbuckets, nsegs;
> < int l2;
> ---
> > int nbuckets, nsegs, l2;
> 437,438c382
> < hdestroy(hashp)
> < HTAB *hashp;
> ---
> > hdestroy(HTAB *hashp)
> 477,479c421,430
> <
> < if (hashp->fp != -1)
> < (void)close(hashp->fp);
> ---
> > if (hashp->tmp_key)
> > free(hashp->tmp_key);
> > if (hashp->tmp_buf)
> > free(hashp->tmp_buf);
> >
> > if (hashp->fp != -1) {
> > if (hashp->save_file)
> > (void)_fsync(hashp->fp);
> > (void)_close(hashp->fp);
> > }
> 497,499c448
> < hash_sync(dbp, flags)
> < const DB *dbp;
> < u_int flags;
> ---
> > hash_sync(const DB *dbp, u_int32_t flags)
> 515a465,466
> > if (hashp->fp != -1 && _fsync(hashp->fp) != 0)
> > return (ERROR);
> 526,527c477
> < flush_meta(hashp)
> < HTAB *hashp;
> ---
> > flush_meta(HTAB *hashp)
> 530c480
> < #if (BYTE_ORDER == LITTLE_ENDIAN)
> ---
> > #if BYTE_ORDER == LITTLE_ENDIAN
> 538c488
> < hashp->HASH_VERSION = HASHVERSION;
> ---
> > hashp->VERSION = HASHVERSION;
> 543c493
> < #if (BYTE_ORDER == LITTLE_ENDIAN)
> ---
> > #if BYTE_ORDER == LITTLE_ENDIAN
> 547,548c497
> < if ((lseek(fp, (off_t)0, SEEK_SET) == -1) ||
> <    ((wsize = write(fp, whdrp, sizeof(HASHHDR))) == -1))
> ---
> > if ((wsize = pwrite(fp, whdrp, sizeof(HASHHDR), (off_t)0)) == -1)
> 574,578c523
> < hash_get(dbp, key, data, flag)
> < const DB *dbp;
> < const DBT *key;
> < DBT *data;
> < u_int flag;
> ---
> > hash_get(const DB *dbp, const DBT *key, DBT *data, u_int32_t flag)
> 591,595c536
> < hash_put(dbp, key, data, flag)
> < const DB *dbp;
> < DBT *key;
> < const DBT *data;
> < u_int flag;
> ---
> > hash_put(const DB *dbp, DBT *key, const DBT *data, u_int32_t flag)
> 601,602c542
> < hashp->error = EINVAL;
> < errno = EINVAL;
> ---
> > hashp->error = errno = EINVAL;
> 614,617c554,555
> < hash_delete(dbp, key, flag)
> < const DB *dbp;
> < const DBT *key;
> < u_int flag; /* Ignored */
> ---
> > hash_delete(const DB *dbp, const DBT *key,
> >     u_int32_t flag) /* Ignored */
> 637,640c575
> < hash_access(hashp, action, key, val)
> < HTAB *hashp;
> < ACTION action;
> < DBT *key, *val;
> ---
> > hash_access(HTAB *hashp, ACTION action, DBT *key, DBT *val)
> 644c579
> < __uint16_t *bp;
> ---
> > u_int16_t *bp;
> 647c582
> < __uint16_t pageno;
> ---
> > u_int16_t pageno;
> 663c598
> < for (bp = (__uint16_t *)rbufp->page, n = *bp++, ndx = 1; ndx < n;)
> ---
> > for (bp = (u_int16_t *)rbufp->page, n = *bp++, ndx = 1; ndx < n;)
> 682c617
> < bp = (__uint16_t *)rbufp->page;
> ---
> > bp = (u_int16_t *)rbufp->page;
> 704c639
> < bp = (__uint16_t *)rbufp->page;
> ---
> > bp = (u_int16_t *)rbufp->page;
> 738c673
> < bp = (__uint16_t *)rbufp->page;
> ---
> > bp = (u_int16_t *)rbufp->page;
> 766,769c701
> < hash_seq(dbp, key, data, flag)
> < const DB *dbp;
> < DBT *key, *data;
> < u_int flag;
> ---
> > hash_seq(const DB *dbp, DBT *key, DBT *data, u_int32_t flag)
> 771c703
> < __uint32_t bucket;
> ---
> > u_int32_t bucket;
> 774c706
> < __uint16_t *bp, ndx;
> ---
> > u_int16_t *bp, ndx;
> 789c721
> <
> ---
> > next_bucket:
> 799c731
> < bp = (__uint16_t *)bufp->page;
> ---
> > bp = (u_int16_t *)bufp->page;
> 804c736
> < if (hashp->cbucket > hashp->MAX_BUCKET) {
> ---
> > if ((u_int32_t)hashp->cbucket > hashp->MAX_BUCKET) {
> 808,809c740,751
> < } else
> < bp = (__uint16_t *)hashp->cpage->page;
> ---
> > } else {
> > bp = (u_int16_t *)hashp->cpage->page;
> > if (flag == R_NEXT || flag == 0) {
> > hashp->cndx += 2;
> > if (hashp->cndx > bp[0]) {
> > hashp->cpage = NULL;
> > hashp->cbucket++;
> > hashp->cndx = 1;
> > goto next_bucket;
> > }
> > }
> > }
> 820c762
> < bp = (__uint16_t *)(bufp->page);
> ---
> > bp = (u_int16_t *)(bufp->page);
> 832a775,776
> > if (hashp->cpage == NULL)
> > return (ERROR);
> 837,843d780
> < ndx += 2;
> < if (ndx > bp[0]) {
> < hashp->cpage = NULL;
> < hashp->cbucket++;
> < hashp->cndx = 1;
> < } else
> < hashp->cndx = ndx;
> 855,857c792,793
> < extern int
> < __expand_table(hashp)
> < HTAB *hashp;
> ---
> > int
> > __expand_table(HTAB *hashp)
> 859c795
> < __uint32_t old_bucket, new_bucket;
> ---
> > u_int32_t old_bucket, new_bucket;
> 881c817
> <    (SEGMENT)calloc(hashp->SGSIZE, sizeof(SEGMENT))) == NULL)
> ---
> >    calloc(hashp->SGSIZE, sizeof(SEGMENT))) == NULL)
> 911,913c847
> < hash_realloc(p_ptr, oldsize, newsize)
> < SEGMENT **p_ptr;
> < int oldsize, newsize;
> ---
> > hash_realloc(SEGMENT **p_ptr, int oldsize, int newsize)
> 926,930c860,861
> < extern __uint32_t
> < __call_hash(hashp, k, len)
> < HTAB *hashp;
> < char *k;
> < int len;
> ---
> > u_int32_t
> > __call_hash(HTAB *hashp, char *k, int len)
> 932c863
> < int n, bucket;
> ---
> > unsigned int n, bucket;
> 947,949c878
> < alloc_segs(hashp, nsegs)
> < HTAB *hashp;
> < int nsegs;
> ---
> > alloc_segs(HTAB *hashp, int nsegs)
> 957c886
> <    (SEGMENT *)calloc(hashp->DSIZE, sizeof(SEGMENT *))) == NULL) {
> ---
> >    calloc(hashp->DSIZE, sizeof(SEGMENT *))) == NULL) {
> 962a892,894
> > hashp->nsegs = nsegs;
> > if (nsegs == 0)
> > return (0);
> 964,965c896
> < if ((store =
> <    (SEGMENT)calloc(nsegs << hashp->SSHIFT, sizeof(SEGMENT))) == NULL) {
> ---
> > if ((store = calloc(nsegs << hashp->SSHIFT, sizeof(SEGMENT))) == NULL) {
> 971c902
> < for (i = 0; i < nsegs; i++, hashp->nsegs++)
> ---
> > for (i = 0; i < nsegs; i++)
> 976c907
> < #if (BYTE_ORDER == LITTLE_ENDIAN)
> ---
> > #if BYTE_ORDER == LITTLE_ENDIAN
> 981,982c912
> < swap_header_copy(srcp, destp)
> < HASHHDR *srcp, *destp;
> ---
> > swap_header_copy(HASHHDR *srcp, HASHHDR *destp)
> 1010,1011c940
> < swap_header(hashp)
> < HTAB *hashp;
> ---
> > swap_header(HTAB *hashp)
>
>>
>> --joel
>>
>