Hi!
> LTP_DATAROOT (test.sh) and tst_dataroot (libltp) define directory
> which holds test data files.
> 
> If LTPROOT is defined, it is $LTPROOT/testcases/data/$TCID
> otherwise it's $CWD/datafiles (where CWD is current working
> directory, before any call to tst_tmdir())

...

>  2.1.3 Subexecutables
>  ^^^^^^^^^^^^^^^^^^^^
> diff --git a/include/tst_resource.h b/include/tst_resource.h
> index a02a313..94c24b7 100644
> --- a/include/tst_resource.h
> +++ b/include/tst_resource.h
> @@ -41,6 +41,9 @@
>  
>  #include "test.h"
>  
> +void tst_dataroot_init(void);

Do we really need to export this function? The only usage for it IMHO
will be the test for the tst_dataroot() function itself. I would rather
see three tests one for each scenario and keep the test library API as
minimal as possible.

> +const char *tst_dataroot(void);
> +

...

> diff --git a/lib/tst_resource.c b/lib/tst_resource.c
> index 54d4795..7c8deee 100644
> --- a/lib/tst_resource.c
> +++ b/lib/tst_resource.c
> @@ -21,9 +21,61 @@
>   * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
>   */
>  
> +#include <pthread.h>
>  #include "tst_resource.h"
>  #include "ltp_priv.h"
>  
> +#ifndef PATH_MAX
> +#ifdef MAXPATHLEN
> +#define PATH_MAX     MAXPATHLEN
> +#else
> +#define PATH_MAX     1024
> +#endif
> +#endif
> +
> +static pthread_mutex_t tmutex = PTHREAD_MUTEX_INITIALIZER;
> +static char dataroot[PATH_MAX];
> +extern char *TCID;
> +
> +void tst_dataroot_init(void)
> +{
> +     const char *ltproot = getenv("LTPROOT");
> +     char curdir[PATH_MAX];
> +     const char *startdir;
> +     int ret;
> +
> +     /* 1. if LTPROOT is set, use $LTPROOT/testcases/data/$TCID
> +      * 2. else if startwd is set by tst_tmdir(), use $STARWD/datafiles
> +      * 3. else use $CWD/datafiles */
> +     if (ltproot) {
> +             ret = snprintf(dataroot, PATH_MAX, "%s/testcases/data/%s",
> +                     ltproot, TCID);
> +     } else {
> +             startdir = tst_get_startwd();
> +             if (startdir[0] == 0) {
> +                     if (getcwd(curdir, PATH_MAX) == NULL)
> +                             tst_brkm(TBROK | TERRNO, NULL,
> +                                     "tst_dataroot getcwd");
> +                     startdir = curdir;
> +             }
> +             ret = snprintf(dataroot, PATH_MAX, "%s/datafiles", startdir);
> +     }
> +
> +     if (ret < 0 || ret >= PATH_MAX)
> +             tst_brkm(TBROK, NULL, "tst_dataroot snprintf: %d", ret);
> +}
> +
> +const char *tst_dataroot(void)
> +{
> +     if (dataroot[0] == 0) {
> +             pthread_mutex_lock(&tmutex);
> +             if (dataroot[0] == 0)
> +                     tst_dataroot_init();
> +             pthread_mutex_unlock(&tmutex);
> +     }

That is a bit too preemptive but generally fine, but I would use
pthread_once() here instead.

> +     return dataroot;
> +}
> +
>  static int file_copy(const char *file, const int lineno,
>                       void (*cleanup_fn)(void), const char *path,
>                    const char *filename, const char *dest)
> @@ -56,15 +108,15 @@ void tst_resource_copy(const char *file, const int 
> lineno,
>               dest = ".";
>  
>       const char *ltproot = getenv("LTPROOT");
> +     const char *dataroot = tst_dataroot();
> +
> +     /* look for data files in $LTP_DATAROOT, $LTPROOT/testcases/bin
> +      * and $CWD */
> +     if (file_copy(file, lineno, cleanup_fn, dataroot, filename, dest))
> +             return;
>  
>       if (ltproot != NULL) {
> -             /* the data are either in testcases/data or testcases/bin */
>               char buf[strlen(ltproot) + 64];
> -
> -             snprintf(buf, sizeof(buf), "%s/testcases/data", ltproot);
> -
> -             if (file_copy(file, lineno, cleanup_fn, buf, filename, dest))
> -                     return;
>               
>               snprintf(buf, sizeof(buf), "%s/testcases/bin", ltproot);

Hmm, maybe we can omit looking into testcases/bin/, since the
directory to store datafiles is well defined now (and there does not
seems to be any testcases that rely on that behavior).

> @@ -72,9 +124,8 @@ void tst_resource_copy(const char *file, const int lineno,
>                       return;
>       }
>  
> +     /* try directory test started in as last resort */
>       const char *startwd = tst_get_startwd();
> -
> -     /* try directory test started int first */
>       if (file_copy(file, lineno, cleanup_fn, startwd, filename, dest))
>               return;
>  
> diff --git a/testcases/lib/test.sh b/testcases/lib/test.sh
> index 10282f4..eecbfba 100644
> --- a/testcases/lib/test.sh
> +++ b/testcases/lib/test.sh
> @@ -144,4 +144,7 @@ export TST_TOTAL="$TST_TOTAL"
>  # Setup LTPROOT, default to current directory if not set
>  if [ -z "$LTPROOT" ]; then
>       export LTPROOT="$PWD"
> +     export LTP_DATAROOT="$LTPROOT/datafiles"
> +else
> +     export LTP_DATAROOT="$LTPROOT/testcases/data/$TCID"
>  fi


Otherwise it looks very good.

-- 
Cyril Hrubis
chru...@suse.cz

------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://p.sf.net/sfu/Zoho
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to