On Wed, Jul 17, 2019, 11:46 PM Gedare Bloom <ged...@rtems.org> wrote:
> On Wed, Jul 17, 2019 at 11:09 AM Vaibhav Gupta <vaibhavgupt...@gmail.com>
> wrote:
> >
> > Thanks for the review!
> > I will re generate the patch with corrections
> >
> > On Wed, Jul 17, 2019 at 10:12 PM Gedare Bloom <ged...@rtems.org> wrote:
> >>
> >> On Wed, Jul 10, 2019 at 1:49 AM Vaibhav Gupta <vaibhavgupt...@gmail.com>
> wrote:
> >> >
> >> > ---
> >> > testsuites/psxtests/Makefile.am | 7 +
> >> > testsuites/psxtests/configure.ac | 1 +
> >> > testsuites/psxtests/psxndbm01/init.c | 260
> +++++++++++++++++++++++++++
> >> > 3 files changed, 268 insertions(+)
> >> > create mode 100644 testsuites/psxtests/psxndbm01/init.c
> >> >
> >> > diff --git a/testsuites/psxtests/Makefile.am
> b/testsuites/psxtests/Makefile.am
> >> > index 59c9f2085b..7d1c0a783c 100755
> >> > --- a/testsuites/psxtests/Makefile.am
> >> > +++ b/testsuites/psxtests/Makefile.am
> >> > @@ -694,6 +694,13 @@ psxmutexattr01_CPPFLAGS = $(AM_CPPFLAGS)
> $(TEST_FLAGS_psxmutexattr01) \
> >> > $(support_includes) -I$(top_srcdir)/include
> >> > endif
> >> >
> >> > +if TEST_psxndbm01
> >> > +psx_tests += psxndbm01
> >> > +psxndbm01_SOURCES = psxndbm01/init.c
> >> > +psxndbm01_CPPFLAGS = $(AM_CPPFLAGS) $(TEST_FLAGS_psxndbm01) \
> >> > + $(support_includes)
> >> > +endif
> >> > +
> >> > if TEST_psxobj01
> >> > psx_tests += psxobj01
> >> > psx_screens += psxobj01/psxobj01.scn
> >> > diff --git a/testsuites/psxtests/configure.ac b/testsuites/psxtests/
> configure.ac
> >> > index 85559e4aa5..07d7ccaf55 100644
> >> > --- a/testsuites/psxtests/configure.ac
> >> > +++ b/testsuites/psxtests/configure.ac
> >> > @@ -110,6 +110,7 @@ RTEMS_TEST_CHECK([psxmsgq02])
> >> > RTEMS_TEST_CHECK([psxmsgq03])
> >> > RTEMS_TEST_CHECK([psxmsgq04])
> >> > RTEMS_TEST_CHECK([psxmutexattr01])
> >> > +RTEMS_TEST_CHECK([psxndbm01])
> >> > RTEMS_TEST_CHECK([psxobj01])
> >> > RTEMS_TEST_CHECK([psxonce01])
> >> > RTEMS_TEST_CHECK([psxpasswd01])
> >> > diff --git a/testsuites/psxtests/psxndbm01/init.c
> b/testsuites/psxtests/psxndbm01/init.c
> >> > new file mode 100644
> >> > index 0000000000..b38900361d
> >> > --- /dev/null
> >> > +++ b/testsuites/psxtests/psxndbm01/init.c
> >> > @@ -0,0 +1,260 @@
> >> > +/**
> >> > + * @file
> >> > + * @brief Test suite for ndbm.h methods
> >> > + */
> >> > +
> >> > +/*
> >> > + * SPDX-License-Identifier: BSD-2-Clause
> >> > + *
> >> > + * Copyright (C) 2019 Vaibhav Gupta
> >> > + *
> >> > + * Redistribution and use in source and binary forms, with or without
> >> > + * modification, are permitted provided that the following conditions
> >> > + * are met:
> >> > + * 1. Redistributions of source code must retain the above copyright
> >> > + * notice, this list of conditions and the following disclaimer.
> >> > + * 2. Redistributions in binary form must reproduce the above
> copyright
> >> > + * notice, this list of conditions and the following disclaimer
> in the
> >> > + * documentation and/or other materials provided with the
> distribution.
> >> > + *
> >> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS "AS IS"
> >> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
> TO, THE
> >> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> PARTICULAR PURPOSE
> >> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
> CONTRIBUTORS BE
> >> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
> OR
> >> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT
> OF
> >> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> BUSINESS
> >> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
> WHETHER IN
> >> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> OTHERWISE)
> >> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> ADVISED OF THE
> >> > + * POSSIBILITY OF SUCH DAMAGE.
> >> > + */
> >> > +
> >> > +#ifdef HAVE_CONFIG_H
> >> > +#include "config.h"
> >> > +#endif
> >> > +
> >> > +/* header files are listed in lexical/lexicographical/alphabetical
> order */
> >> > +
> >> > +#include <errno.h>
> >> > +#include <fcntl.h> /* contains definitions of `open_flags` */
> >> > +#include <limits.h>
> >> > +#include <ndbm.h> /* conatains declarations of ndbm methods */
> >> > +#include <stddef.h>
> >> > +#include <stdint.h>
> >> > +#include <stdio.h>
> >> > +#include <sys/stat.h> /* contains definitions of `file_mode` */
> >> > +#include <string.h>
> >> > +#include <rtems/test.h>
> >> > +#include <tmacros.h>
> >> > +
> >> > +const char rtems_test_name[] = "PSXNDBM 01";
> >> > +
> >> > +#define NAME "VARoDeK"
> >> > +#define PHONE_NO "123-321-777-888"
> >> > +#define DB_NAME "phones_test"
> >> > +#define NAME2 "VG"
> >> > +#define PHONE_NO2 "321-123-888-777"
> >> > +
> >> > +/* forward declarations to avoid warnings */
> >> > +rtems_task Init(rtems_task_argument ignored);
> >> > +
> >> > +rtems_task Init(rtems_task_argument ignored)
> >> > +{
> >> > + datum name = { NAME , sizeof( NAME ) };
> >> > + datum put_phone_no = { PHONE_NO, sizeof( PHONE_NO ) };
> >> > + datum get_phone_no, key;
> >> > +
> >> > + datum name2 = { NAME2 , sizeof( NAME2 ) };
> >> > + datum put_phone_no2 = { PHONE_NO2 , sizeof( PHONE_NO2 ) };
> >> > +
> >> > + int i;
> >> > + char *test_strings;
> >> > +
> >> > + DBM *db;
> >> > +
> >> > + TEST_BEGIN();
> >> > +
> >> > +/* A Simple test to check if ndbm methods are call-able */
> >> > +
> >> > +/* Store a simple data
> >> > + * This record (procedure) will be helpful in further tests
> >> > + */
> >> > +
> >> > + puts( "Open Database." );
> >> > + db = dbm_open( DB_NAME , O_RDWR | O_CREAT | O_TRUNC , S_IRWXU );
> >> Remove the extra blank space character before the comma.
> >>
> >> > + rtems_test_assert( db != NULL );
> >> > +
> >> > + puts( "Store Records in Database." );
> >> > + dbm_store( db , name , put_phone_no , DBM_INSERT );
> >> ditto, and more below.
> >>
> >> > +
> >> > + puts( "Fetch Records from Database and check." );
> >> > + get_phone_no = dbm_fetch( db , name );
> >> > + rtems_test_assert( strcmp( (const char*)get_phone_no.dptr ,
> PHONE_NO ) == 0 );
> >>
> >> > +
> >> > + puts( "Close Database." );
> >> > + dbm_close( db );
> >> > +
> >> > +/* dbm_open() */
> >> I would like this to be more clearly about documenting what is
> >> happening, as it looks from casual glance to be commented-out code one
> >> wonders what it does here.
> >>
> >> > +
> >> > + puts( "\nTestcases for `dbm_open()`" );
> >> I would avoid the use of the back-ticks ` characters. Historically,
> >> they can cause problems in *nix systems.
> >>
> >> > +
> >> > +/* The `DB_NAME` is already created, hence `O_RDWR | O_EXCL` should
> fail. */
> >> > +
> >> > + puts( "Use `O_CREAT | O_EXCL` to open existing file and confirm
> error." );
> >> > + db = dbm_open( DB_NAME , O_RDWR | O_CREAT | O_EXCL , S_IRWXU );
> >> > + rtems_test_assert( db == NULL );
> >> > + rtems_test_assert( errno == EEXIST );
> >> > +
> >> > +/* Some implementations use 3 characters for a suffix and others use
> >> > + * 4 characters for a suffix, applications should ensure that the
> maximum
> >> > + * portable pathname length passed to dbm_open() is no greater than
> >> > + * {PATH_MAX}-4 bytes, with the last component of the pathname no
> greater
> >> > + * than {NAME_MAX}-4 bytes.
> >> > + */
> >> > +
> >> > +/* inside `ndbm.h` ; `#define DBM_SUFFIX ".db"` ;
> >> > + * 2 alphabtes and 1 period, hence 3 characters are used for suffix
> >> typo: alphabets (or alphabetical)
> >>
> >> > + * in this implementation.
> >> > + */
> >> > +
> >> > + puts( "Use path name larger than `{PATH_MAX}-3 bytes.` and confirm
> error." );
> >> > + test_strings = (char*)malloc( ( PATH_MAX - 2 ) );
> >> the parentheses around PATH_MAX - 2 are redundant
> >>
> >> > + for ( i = 0 ; i < ( PATH_MAX - 3 ) ; i++ ){
> >> space before {.
> >> https://devel.rtems.org/wiki/Developer/Coding/Conventions#Formatting
> >>
> >> > + test_strings[i] = 'r';
> >> > + }
> >> > + test_strings[i] = '\0';
> >> > + db = dbm_open( (const char*)test_strings , O_RDWR | O_CREAT |
> O_TRUNC , S_IRWXU );
> >> This line is > 80 characters. Please break it apart as per
> >> https://devel.rtems.org/wiki/Developer/Coding/80_characters_per_line
> >>
> >> > + rtems_test_assert( db == NULL );
> >> > + rtems_test_assert( errno == ENAMETOOLONG );
> >> > + free( test_strings );
> >> > +
> >> > +/* database opened for write-only access opens the files for read and
> >> > + * write access or it will fail.
> >> > + */
> >> > +
> >> > +/* Implemenation of __hash_open in newlib does not support
> `O_WRONLY` */
> >> typo: Implementation
> >>
> >> > +
> >> > + puts( "Open file with write access only and confirm error." );
> >> > + db = dbm_open( DB_NAME , O_WRONLY , S_IRWXU );
> >> > + rtems_test_assert( db == NULL );
> >> > + rtems_test_assert( errno == EINVAL );
> >> > +
> >> > +/* dbm_store() */
> >> > +
> >> > + puts( "\nTestcases for `dbm_store()`" );
> >> > + db = dbm_open( DB_NAME , O_RDWR , S_IRWXU );
> >> > + rtems_test_assert( db != NULL );
> >> > +
> >> > + puts( "Insert new record with same key using `DBM_INSERT` mode and
> confirm error." );
> >> > + rtems_test_assert( dbm_store( db , name , put_phone_no2 ,
> DBM_INSERT ) == 1 );
> >> also too long. fix all lines > 80 characters
> >>
> >> > + get_phone_no = dbm_fetch( db , name );
> >> > + rtems_test_assert( strcmp( (const char*)get_phone_no.dptr ,
> PHONE_NO ) == 0 );
> >> > +
> >> > + puts( "Insert new record with same key using `DBM_REPLACE` mode
> and confirm changes." );
> >> > + rtems_test_assert( dbm_store( db , name , put_phone_no2 ,
> DBM_REPLACE ) == 0 );
> >> > + get_phone_no = dbm_fetch( db , name );
> >> > + rtems_test_assert( strcmp( (const char*)get_phone_no.dptr ,
> PHONE_NO2 ) == 0 );
> >> > +
> >> > +/* Revert for next tests*/
> >> > +
> >> > + rtems_test_assert( dbm_store( db , name , put_phone_no ,
> DBM_REPLACE ) == 0 );
> >> > +
> >> > + puts( "Store a new record and confirm, total number of records is
> successful 2." );
> >> > + rtems_test_assert( dbm_store( db , name2 , put_phone_no2 ,
> DBM_INSERT ) == 0 );
> >> > + for(key = dbm_firstkey( db ) , i = 0; key.dptr != NULL; key =
> dbm_nextkey( db ) , i++ );
> >> add spaces around ( after for:
> >>
> >> > + rtems_test_assert( i == 2);
> >> spacing
> >>
> >> > + dbm_close( db );
> >> > +
> >> > +/* dbm_fetch() */
> >> > +
> >> > + puts( "\nTestcases for `dbm_fetch()`" );
> >> > + db = dbm_open( DB_NAME , O_RDONLY , S_IRWXU );
> >> > + rtems_test_assert( db != NULL );
> >> > +
> >> > + puts( "Fetch existing records and confirm results." );
> >> > + get_phone_no = dbm_fetch( db , name );
> >> > + rtems_test_assert( strcmp( (const char*)get_phone_no.dptr ,
> PHONE_NO ) == 0 );
> >> > + get_phone_no = dbm_fetch( db , name2 );
> >> > + rtems_test_assert( strcmp( (const char*)get_phone_no.dptr ,
> PHONE_NO2 ) == 0 );
> >> > +
> >> > + puts( "Fetch non-existing record and conform error." );
> >> typo: confirm
> >>
> >> > + test_strings = (char*)malloc(6);
> >> > + strcpy( test_strings , "Hello" );
> >> suggest you use strncpy.
> >>
> >> > + key.dptr = test_strings;
> >> > + key.dsize = sizeof( test_strings );
> >> > + get_phone_no = dbm_fetch( db , key );
> >> > + rtems_test_assert( get_phone_no.dptr == NULL );
> >> > + dbm_close( db );
> >> > +
> >> > +/* dbm_delete() */
> >> > +
> >> > + puts( "\nTestcases for `dbm_delete()`" );
> >> > + db = dbm_open( DB_NAME , O_RDWR , S_IRWXU );
> >> > + rtems_test_assert( db != NULL );
> >> > +
> >> > + puts( "Delte non-existing record and confirm error." );
> >> typo: Delete
> >>
> >> > + rtems_test_assert( dbm_delete( db , key ) != 0 );
> >> > + free( test_strings );
> >> > + for(key = dbm_firstkey( db ) , i = 0; key.dptr != NULL; key =
> dbm_nextkey( db ) , i++ );
> >> > + rtems_test_assert( i == 2);
> >> since you are repeating this logic many times, you might consider
> >> writing a helper function to count the records in the db. It will make
> >> the test easier to read too.
> >
> > Sure, I must confirm for this from Newlib first. Or else I can make
> > a user-defined function in a tesuite itself for this purpose.
> > .
> > It will take database name as an argument and count number of
> > records.
> This has nothing to do with newlib. It is about how to write good
> code. You should define top of this file something like
>
> static int count_records( DBM *db )
> {
> int i = 0;
> ...
> return i;
> }
>
Oh, sorry, my mistake. I mis-read it. I read that you were suggesting to
write function in db.h header.
Yeah, i will write a helper function for this.
>
> >>
> >> > +
> >> > + puts("Delete existing record and confirm, total number of records
> is successful 1.");
> >> > + rtems_test_assert( dbm_delete( db , name ) == 0 );
> >> > + for(key = dbm_firstkey( db ) , i = 0; key.dptr != NULL; key =
> dbm_nextkey( db ) , i++ );
> >> > + rtems_test_assert( i == 1);
> >> > +
> >> > + puts("Confirm if correct record is deleted.");
> >> > + get_phone_no = dbm_fetch( db , name );
> >> > + rtems_test_assert( get_phone_no.dptr == NULL );
> >> > +
> >> > +/* record returned by `dbm_firstkey()` should be the only record
> >> > + * left, this should be checked to confirm correct working of
> >> > + * `dbm_firstkey()`.
> >> > + * Check if the data is not corrupted after usage of `dbm_delete()`
> >> > + */
> >> > +
> >> > + puts( "Check if the data is not corrupted after usage of
> `dbm_delete()`." );
> >> > + get_phone_no = dbm_fetch( db , dbm_firstkey( db ) );
> >> > + rtems_test_assert( strcmp( (const char*)get_phone_no.dptr ,
> PHONE_NO2 ) == 0 );
> >> > +
> >> > +/* Empty the database and then try to use `dbm_firstkey()`, the
> >> > + * dptr pointer should point to NULL.
> >> > + */
> >> > +
> >> > + puts( "Empty records in database and check results of
> `dbm_firstkey()`." );
> >> > + rtems_test_assert( dbm_delete( db , dbm_firstkey( db ) ) == 0 );
> >> > + key = dbm_firstkey( db );
> >> > + rtems_test_assert( key.dptr == NULL );
> >> > + dbm_close( db );
> >> > +
> >> > +/* `dbm_firstkey()` */
> >> > +
> >> > + puts( "\nTestcases for `dbm_firstkey()`" );
> >> > + puts( "All cases tested while performing above tests." );
> >> > +
> >> > +/* `dbm_nextkey()` */
> >> > +
> >> > + puts( "\nTestcases for `dbm_nextkey()`" );
> >> > + puts( "All cases tested while performing above tests." );
> >> > +
> >> these are not necessary print statements then. you can just make it a
> >> comment somewhere in here that those functions are tested already.
> >>
> >> > + TEST_END();
> >> > + rtems_test_exit(0);
> >> > +}
> >> > +
> >> > +/* NOTICE: the clock driver is explicitly disabled */
> >> > +
> >> > +#define CONFIGURE_APPLICATION_DOES_NOT_NEED_CLOCK_DRIVER
> >> > +#define CONFIGURE_APPLICATION_NEEDS_CONSOLE_DRIVER
> >> > +
> >> > +#define CONFIGURE_MAXIMUM_TASKS 1
> >> > +
> >> > +#define CONFIGURE_LIBIO_MAXIMUM_FILE_DESCRIPTORS 6
> >> > +
> >> > +#define CONFIGURE_RTEMS_INIT_TASKS_TABLE
> >> > +
> >> > +#define CONFIGURE_INITIAL_EXTENSIONS RTEMS_TEST_INITIAL_EXTENSION
> >> > +
> >> > +#define CONFIGURE_INIT
> >> > +#include <rtems/confdefs.h>
> >> > +/* end of file */
> >> > --
> >> > 2.21.0
> >> >
>
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
