* [ptest-runner][PATCH 2/4] main code: fix clang warnings
2019-07-17 18:35 [ptest-runner][PATCH 1/4] utils: ensure child can be session leader Randy MacLeod
@ 2019-07-17 18:35 ` Randy MacLeod
2019-07-17 18:35 ` [ptest-runner][PATCH 3/4] tests: " Randy MacLeod
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Randy MacLeod @ 2019-07-17 18:35 UTC (permalink / raw)
To: yocto, anibal.limon
Fix basic errors found when building with the clang compiler
with the option -Weverything. There are a few warnings that remain
that are not variable casting, macro fixes, or similarily simple
changes.
Makefile: add -lutil for 'check' builds for clang/gcc
builds.
Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
---
Makefile | 4 +++-
main.c | 9 ++++-----
ptest_list.c | 17 +++++++++++++++--
ptest_list.h | 29 ++++++++++++++++++++++-------
utils.c | 38 +++++++++++++++++++++-----------------
utils.h | 4 ++--
6 files changed, 67 insertions(+), 34 deletions(-)
diff --git a/Makefile b/Makefile
index 439eb79..c92261b 100644
--- a/Makefile
+++ b/Makefile
@@ -3,6 +3,8 @@ MEMCHECK=$(shell echo $$MEMCHECK)
CC=cc
CFLAGS=-std=gnu99 -pedantic -Wall -Werror -I .
+# CC=clang
+# CFLAGS=-std=gnu99 -Weverything -I .
ifeq ($(RELEASE), 1)
CFLAGS+= -O2 -DRELEASE
else
@@ -22,7 +24,7 @@ TEST_SOURCES=tests/main.c tests/ptest_list.c tests/utils.c $(BASE_SOURCES)
TEST_OBJECTS=$(TEST_SOURCES:.c=.o)
TEST_EXECUTABLE=ptest-runner-test
TEST_LDFLAGS=-lm -lrt -lpthread
-TEST_LIBSTATIC=-lcheck -lsubunit
+TEST_LIBSTATIC=-lcheck -lsubunit -lutil
TEST_DATA=$(shell echo `pwd`/tests/data)
diff --git a/main.c b/main.c
index 83cd8f2..01d60f7 100644
--- a/main.c
+++ b/main.c
@@ -93,7 +93,7 @@ main(int argc, char *argv[])
}
- opts.exclude = malloc(ptest_exclude_num * sizeof(char));
+ opts.exclude = malloc((size_t)ptest_exclude_num * sizeof(char));
CHECK_ALLOCATION(opts.exclude, 1, 1);
i = 0;
@@ -116,7 +116,7 @@ main(int argc, char *argv[])
case 'h':
print_usage(stdout, argv[0]);
exit(0);
- break;
+ /* break; not needed, not reachable after exit() */
case 'x':
free(opts.xml_filename);
opts.xml_filename = strdup(optarg);
@@ -125,13 +125,12 @@ main(int argc, char *argv[])
default:
print_usage(stdout, argv[0]);
exit(1);
- break;
}
}
ptest_num = argc - optind;
if (ptest_num > 0) {
- size_t size = ptest_num * sizeof(char *);
+ size_t size = sizeof(char *) * (unsigned int) ptest_num;
opts.ptests = calloc(1, size);
CHECK_ALLOCATION(opts.ptests, size, 1);
@@ -163,7 +162,7 @@ main(int argc, char *argv[])
}
run = filter_ptests(head, opts.ptests, ptest_num);
- CHECK_ALLOCATION(run, ptest_num, 1);
+ CHECK_ALLOCATION(run, (size_t) ptest_num, 1);
ptest_list_free_all(head);
}
diff --git a/ptest_list.c b/ptest_list.c
index d48349f..a5632f8 100644
--- a/ptest_list.c
+++ b/ptest_list.c
@@ -29,8 +29,21 @@
#include "utils.h"
#include "ptest_list.h"
-#define VALIDATE_PTR_RINT(ptr) if (ptr == NULL) { errno = EINVAL; return -1; }
-#define VALIDATE_PTR_RNULL(ptr) if (ptr == NULL) { errno = EINVAL; return NULL; }
+#define VALIDATE_PTR_RINT(ptr) \
+ do { \
+ if (ptr == NULL) { \
+ errno = EINVAL; \
+ return -1; \
+ } \
+ } while (0)
+
+#define VALIDATE_PTR_RNULL(ptr) \
+ do { \
+ if (ptr == NULL) { \
+ errno = EINVAL; \
+ return NULL; \
+ } \
+ } while (0)
struct ptest_list *
ptest_list_alloc()
diff --git a/ptest_list.h b/ptest_list.h
index b4b1ac6..e1caffc 100644
--- a/ptest_list.h
+++ b/ptest_list.h
@@ -21,13 +21,28 @@
* Aníbal Limón <anibal.limon@intel.com>
*/
-#ifndef _PTEST_RUNNER_LIST_H_
-#define _PTEST_RUNNER_LIST_H_
+#ifndef PTEST_RUNNER_LIST_H
+#define PTEST_RUNNER_LIST_H
-#define PTEST_LIST_FREE_CLEAN(x) { ptest_list_free(x); x = NULL; }
-#define PTEST_LIST_FREE_ALL_CLEAN(x) { ptest_list_free_all(x); x = NULL; }
+#define FLUSH_PRINTF(...) \
+ do { \
+ printf(__VA_ARGS__); \
+ fflush(stdout); \
+ } while (0)
-#define PTEST_LIST_ITERATE_START(head, p) for (p = head->next; p != NULL; p = p->next) {
+#define PTEST_LIST_FREE_CLEAN(x) \
+ do { \
+ ptest_list_free(x); \
+ x = NULL; \
+ } while (0)
+
+#define PTEST_LIST_FREE_ALL_CLEAN(x) \
+ do { \
+ ptest_list_free_all(x); \
+ x = NULL; \
+ } while (0)
+
+#define PTEST_LIST_ITERATE_START(head, p) for (p = head->next; p != NULL; p = p->next) {
#define PTEST_LIST_ITERATE_END }
#include <sys/stat.h>
@@ -40,7 +55,7 @@ struct ptest_list {
struct ptest_list *prev;
};
-extern struct ptest_list *ptest_list_alloc();
+extern struct ptest_list *ptest_list_alloc(void);
extern void ptest_list_free(struct ptest_list *);
extern int ptest_list_free_all(struct ptest_list *);
@@ -50,4 +65,4 @@ extern struct ptest_list *ptest_list_search_by_file(struct ptest_list *, char *,
extern struct ptest_list *ptest_list_add(struct ptest_list *, char *, char *);
extern struct ptest_list *ptest_list_remove(struct ptest_list *, char *, int);
-#endif // _PTEST_RUNNER_LIST_H_
+#endif // PTEST_RUNNER_LIST_H
diff --git a/utils.c b/utils.c
index f11ce39..92654bf 100644
--- a/utils.c
+++ b/utils.c
@@ -22,7 +22,7 @@
* Aníbal Limón <anibal.limon@intel.com>
*/
-#define _GNU_SOURCE
+#define _GNU_SOURCE
#include <stdio.h>
@@ -84,7 +84,7 @@ get_available_ptests(const char *dir)
int n, i;
struct dirent **namelist;
int fail;
- int saved_errno;
+ int saved_errno = -1; /* Initalize to invalid errno. */
do
{
@@ -190,9 +190,9 @@ print_ptests(struct ptest_list *head, FILE *fp)
} else {
struct ptest_list *n;
fprintf(fp, PRINT_PTESTS_AVAILABLE);
- PTEST_LIST_ITERATE_START(head, n);
+ PTEST_LIST_ITERATE_START(head, n)
fprintf(fp, "%s\t%s\n", n->ptest, n->run_ptest);
- PTEST_LIST_ITERATE_END;
+ PTEST_LIST_ITERATE_END
return 0;
}
}
@@ -201,7 +201,7 @@ struct ptest_list *
filter_ptests(struct ptest_list *head, char **ptests, int ptest_num)
{
struct ptest_list *head_new = NULL, *n;
- int fail = 0, i, saved_errno;
+ int fail = 0, i, saved_errno = 0;
do {
if (head == NULL || ptests == NULL || ptest_num <= 0) {
@@ -260,7 +260,7 @@ close_fds(void)
getrlimit(RLIMIT_NOFILE, &curr_lim);
int fd;
- for (fd=3; fd < curr_lim.rlim_cur; fd++) {
+ for (fd=3; fd < (int)curr_lim.rlim_cur; fd++) {
(void) close(fd);
}
}
@@ -277,10 +277,14 @@ run_child(char *run_ptest, int fd_stdout, int fd_stderr)
dup2(fd_stdout, STDOUT_FILENO);
// XXX: Redirect stderr to stdout to avoid buffer ordering problems.
dup2(fd_stdout, STDERR_FILENO);
+
+ /* since it isn't use by the child, close(fd_stderr) ? */
+ close(fd_stderr); /* try using to see if this fixes bash run-read. rwm todo */
close_fds();
+
execv(run_ptest, argv);
- exit(1);
+ /* exit(1); not needed? */
}
static inline int
@@ -293,7 +297,7 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
int looping = 1;
int r;
- int status;
+ int status = -1;
int waitflags;
pfds[0].fd = fds[0];
@@ -332,13 +336,13 @@ wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
if (pfds[0].revents != 0) {
while ((n = read(fds[0], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0)
- fwrite(buf, n, 1, fps[0]);
+ fwrite(buf, (size_t)n, 1, fps[0]);
}
if (pfds[1].revents != 0) {
while ((n = read(fds[1], buf, WAIT_CHILD_BUF_MAX_SIZE)) > 0) {
fflush(fps[0]);
- fwrite(buf, n, 1, fps[1]);
+ fwrite(buf, (size_t)n, 1, fps[1]);
fflush(fps[1]);
}
}
@@ -376,7 +380,7 @@ setup_slave_pty(FILE *fp) {
/* If the tty group does not exist, don't change the
* group on the slave pty, only the owner
*/
- gid = -1;
+ gid = (gid_t)-1;
}
/* chown/chmod the corresponding pty, if possible.
@@ -403,7 +407,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
const char *progname, FILE *fp, FILE *fp_stderr)
{
int rc = 0;
- FILE *xh;
+ FILE *xh = NULL;
struct ptest_list *p;
char stime[GET_STIME_BUF_SIZE];
@@ -413,7 +417,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
int pipefd_stderr[2];
int timeouted;
time_t sttime, entime;
- int duration;
+ time_t duration;
int slave;
int pgid = -1;
@@ -434,7 +438,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
break;
}
fprintf(fp, "START: %s\n", progname);
- PTEST_LIST_ITERATE_START(head, p);
+ PTEST_LIST_ITERATE_START(head, p)
char *ptest_dir = strdup(p->run_ptest);
if (ptest_dir == NULL) {
rc = -1;
@@ -495,17 +499,17 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
fprintf(fps[0], "\nERROR: Exit status is %d\n", status);
rc += 1;
}
- fprintf(fps[0], "DURATION: %d\n", duration);
+ fprintf(fps[0], "DURATION: %d\n", (int) duration);
if (timeouted)
fprintf(fps[0], "TIMEOUT: %s\n", ptest_dir);
if (opts.xml_filename)
- xml_add_case(xh, status, ptest_dir, timeouted, duration);
+ xml_add_case(xh, status, ptest_dir, timeouted, (int) duration);
fprintf(fp, "END: %s\n", ptest_dir);
fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, entime));
}
- PTEST_LIST_ITERATE_END;
+ PTEST_LIST_ITERATE_END
fprintf(fp, "STOP: %s\n", progname);
close(pipefd_stdout[0]); close(pipefd_stdout[1]);
diff --git a/utils.h b/utils.h
index 6175ed1..f6a56f1 100644
--- a/utils.h
+++ b/utils.h
@@ -21,8 +21,8 @@
* Aníbal Limón <anibal.limon@intel.com>
*/
-#ifndef _PTEST_RUNNER_UTILS_H_
-#define _PTEST_RUNNER_UTILS_H_
+#ifndef PTEST_RUNNER_UTILS_H
+#define PTEST_RUNNER_UTILS_H
#include "ptest_list.h"
--
2.17.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [ptest-runner][PATCH 3/4] tests: fix clang warnings.
2019-07-17 18:35 [ptest-runner][PATCH 1/4] utils: ensure child can be session leader Randy MacLeod
2019-07-17 18:35 ` [ptest-runner][PATCH 2/4] main code: fix clang warnings Randy MacLeod
@ 2019-07-17 18:35 ` Randy MacLeod
2019-07-17 18:35 ` [ptest-runner][PATCH 4/4] Fix additional warnings when using clang Randy MacLeod
2019-07-21 19:22 ` [ptest-runner][PATCH 1/4] utils: ensure child can be session leader Anibal Limon
3 siblings, 0 replies; 7+ messages in thread
From: Randy MacLeod @ 2019-07-17 18:35 UTC (permalink / raw)
To: yocto, anibal.limon
Make tests build using: clang -Weverything
There are a few warnings that remain that are not
variable casting or macro fixes.
Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
---
tests/main.c | 5 +----
tests/utils.c | 19 +++++++++----------
2 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/tests/main.c b/tests/main.c
index c3b4da5..e1a8b69 100644
--- a/tests/main.c
+++ b/tests/main.c
@@ -62,13 +62,10 @@ main(int argc, char *argv[])
opts_directory = strdup(optarg);
break;
case 'h':
- print_usage(stdout, argv[0]);
- exit(0);
- break;
+ /* fall though !! */
default:
print_usage(stdout, argv[0]);
exit(1);
- break;
}
}
diff --git a/tests/utils.c b/tests/utils.c
index 3ba64d6..571d488 100644
--- a/tests/utils.c
+++ b/tests/utils.c
@@ -33,7 +33,6 @@
#include "utils.h"
#define PRINT_PTEST_BUF_SIZE 8192
-#define PRINT_PTEST_MAX_LINE 512
extern char *opts_directory;
@@ -247,15 +246,15 @@ END_TEST
static int
filecmp(FILE *fp1, FILE *fp2)
{
- char f1, f2;
- while (1) {
- int end = 0;
- if ((f1 = getc(fp1)) == EOF) end++;
- if ((f2 = getc(fp2)) == EOF) end++;
-
- if (end == 2) return 0;
- if (end == 1) return 1;
- if (f1 != f2) return 2;
+ int f1, f2;
+ while (1) {
+ int end = 0;
+ if ((f1 = getc(fp1)) == EOF) end++;
+ if ((f2 = getc(fp2)) == EOF) end++;
+
+ if (end == 2) return 0;
+ if (end == 1) return 1;
+ if (f1 != f2) return 2;
}
}
--
2.17.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [ptest-runner][PATCH 4/4] Fix additional warnings when using clang
2019-07-17 18:35 [ptest-runner][PATCH 1/4] utils: ensure child can be session leader Randy MacLeod
2019-07-17 18:35 ` [ptest-runner][PATCH 2/4] main code: fix clang warnings Randy MacLeod
2019-07-17 18:35 ` [ptest-runner][PATCH 3/4] tests: " Randy MacLeod
@ 2019-07-17 18:35 ` Randy MacLeod
2019-08-01 18:19 ` Anibal Limon
2019-07-21 19:22 ` [ptest-runner][PATCH 1/4] utils: ensure child can be session leader Anibal Limon
3 siblings, 1 reply; 7+ messages in thread
From: Randy MacLeod @ 2019-07-17 18:35 UTC (permalink / raw)
To: yocto, anibal.limon
Drop unused function parameters in wait_child().
The remaining warning in the top dir code is:
utils.c:25:9: warning: macro name is a reserved identifier [-Wreserved-id-macro]
#define _GNU_SOURCE
and it's not clear how to deal with that in a portable manner.
Drop unused variable in analizer code.
Rename analizer to analyzer.
Avoid program scope global by adding a set function for
the opts directory variable.
Free strdup()ed memory before exit.
Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
---
tests/main.c | 13 ++++++++-----
tests/ptest_list.c | 2 ++
tests/utils.c | 22 ++++++++++++++--------
utils.c | 6 ++----
utils.h | 2 ++
5 files changed, 28 insertions(+), 17 deletions(-)
diff --git a/tests/main.c b/tests/main.c
index e1a8b69..1344bc0 100644
--- a/tests/main.c
+++ b/tests/main.c
@@ -38,14 +38,14 @@ static SuiteFunction *suites[] = {
NULL,
};
+extern void set_opts_dir(char *);
+
static inline void
print_usage(FILE *stream, char *progname)
{
fprintf(stream, "Usage: %s <-d directory>\n", progname);
}
-char *opts_directory;
-
int
main(int argc, char *argv[])
{
@@ -54,12 +54,12 @@ main(int argc, char *argv[])
int number_failed;
SuiteFunction *sf;
- opts_directory = NULL;
+ char *opts_dir = NULL;
while ((opt = getopt(argc, argv, "d:t:h")) != -1) {
switch (opt) {
case 'd':
- opts_directory = strdup(optarg);
+ opts_dir = strdup(optarg);
break;
case 'h':
/* fall though !! */
@@ -69,10 +69,11 @@ main(int argc, char *argv[])
}
}
- if (opts_directory == NULL) {
+ if (opts_dir == NULL) {
print_usage(stdout, argv[0]);
exit(1);
}
+ set_opts_dir(opts_dir);
i = 0;
number_failed = 0;
@@ -88,6 +89,8 @@ main(int argc, char *argv[])
i++;
sf = suites[i];
}
+ set_opts_dir(NULL);
+ free(opts_dir);
return number_failed;
}
diff --git a/tests/ptest_list.c b/tests/ptest_list.c
index e0ec276..081f027 100644
--- a/tests/ptest_list.c
+++ b/tests/ptest_list.c
@@ -29,6 +29,8 @@
#include "ptest_list.h"
+extern Suite *ptest_list_suite(void);
+
static int ptests_num = 6;
static char *ptest_names[] = {
"python",
diff --git a/tests/utils.c b/tests/utils.c
index 571d488..4fa4609 100644
--- a/tests/utils.c
+++ b/tests/utils.c
@@ -32,9 +32,15 @@
#include "ptest_list.h"
#include "utils.h"
+Suite *utils_suite(void);
+
#define PRINT_PTEST_BUF_SIZE 8192
-extern char *opts_directory;
+static char *opts_directory = NULL;
+
+void set_opts_dir(char * od) {
+ opts_directory = od;
+}
static char *ptests_found[] = {
"bash",
@@ -68,7 +74,7 @@ find_word(int *found, const char *line, const char *word)
}
static void test_ptest_expected_failure(struct ptest_list *, const int, char *,
- void (*h_analizer)(const int, FILE *, FILE *));
+ void (*h_analyzer)(const int, FILE *));
START_TEST(test_get_available_ptests)
{
@@ -189,7 +195,7 @@ START_TEST(test_run_ptests)
END_TEST
static void
-search_for_timeout_and_duration(const int rp, FILE *fp_stdout, FILE *fp_stderr)
+search_for_timeout_and_duration(const int rp, FILE *fp_stdout)
{
const char *timeout_str = "TIMEOUT";
const char *duration_str = "DURATION";
@@ -218,7 +224,7 @@ START_TEST(test_run_timeout_duration_ptest)
END_TEST
static void
-search_for_fail(const int rp, FILE *fp_stdout, FILE *fp_stderr)
+search_for_fail(const int rp, FILE *fp_stdout)
{
const char *fail_str = "ERROR: Exit status is";
char line_buf[PRINT_PTEST_BUF_SIZE];
@@ -284,7 +290,7 @@ START_TEST(test_xml_fail)
END_TEST
Suite *
-utils_suite()
+utils_suite(void)
{
Suite *s;
TCase *tc_core;
@@ -308,7 +314,7 @@ utils_suite()
static void
test_ptest_expected_failure(struct ptest_list *head, const int timeout, char *progname,
- void (*h_analizer)(const int, FILE *, FILE *))
+ void (*h_analyzer)(const int, FILE *))
{
char *buf_stdout;
size_t size_stdout = PRINT_PTEST_BUF_SIZE;
@@ -329,9 +335,9 @@ test_ptest_expected_failure(struct ptest_list *head, const int timeout, char *pr
struct ptest_options opts = EmptyOpts;
opts.timeout = timeout;
- h_analizer(
+ h_analyzer(
run_ptests(filtered, opts, progname, fp_stdout, fp_stderr),
- fp_stdout, fp_stderr
+ fp_stdout
);
PTEST_LIST_FREE_ALL_CLEAN(filtered);
diff --git a/utils.c b/utils.c
index 92654bf..a8ba190 100644
--- a/utils.c
+++ b/utils.c
@@ -288,8 +288,7 @@ run_child(char *run_ptest, int fd_stdout, int fd_stderr)
}
static inline int
-wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
- int timeout, int *fds, FILE **fps, int *timeouted)
+wait_child(pid_t pid, int timeout, int *fds, FILE **fps, int *timeouted)
{
struct pollfd pfds[2];
struct timespec sentinel;
@@ -490,8 +489,7 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts,
fprintf(fp, "%s\n", get_stime(stime, GET_STIME_BUF_SIZE, sttime));
fprintf(fp, "BEGIN: %s\n", ptest_dir);
- status = wait_child(ptest_dir, p->run_ptest, child,
- opts.timeout, fds, fps, &timeouted);
+ status = wait_child(child, opts.timeout, fds, fps, &timeouted);
entime = time(NULL);
duration = entime - sttime;
diff --git a/utils.h b/utils.h
index f6a56f1..aa53707 100644
--- a/utils.h
+++ b/utils.h
@@ -53,4 +53,6 @@ extern FILE *xml_create(int, char *);
extern void xml_add_case(FILE *, int, const char *, int, int);
extern void xml_finish(FILE *);
+void set_opts_dir(char * od);
+
#endif
--
2.17.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [ptest-runner][PATCH 4/4] Fix additional warnings when using clang
2019-07-17 18:35 ` [ptest-runner][PATCH 4/4] Fix additional warnings when using clang Randy MacLeod
@ 2019-08-01 18:19 ` Anibal Limon
2019-08-01 18:34 ` Randy MacLeod
0 siblings, 1 reply; 7+ messages in thread
From: Anibal Limon @ 2019-08-01 18:19 UTC (permalink / raw)
To: Randy MacLeod, Richard Purdie; +Cc: yocto
[-- Attachment #1: Type: text/plain, Size: 7080 bytes --]
Hi Randy,
Just push your clang fixes to master and created v2.3.2 tag, feel free to
send a recipe upgrade.
Regards,
Anibal
On Wed, 17 Jul 2019 at 13:36, Randy MacLeod <Randy.MacLeod@windriver.com>
wrote:
> Drop unused function parameters in wait_child().
>
> The remaining warning in the top dir code is:
> utils.c:25:9: warning: macro name is a reserved identifier
> [-Wreserved-id-macro]
> #define _GNU_SOURCE
> and it's not clear how to deal with that in a portable manner.
>
> Drop unused variable in analizer code.
> Rename analizer to analyzer.
>
> Avoid program scope global by adding a set function for
> the opts directory variable.
>
> Free strdup()ed memory before exit.
>
> Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
> ---
> tests/main.c | 13 ++++++++-----
> tests/ptest_list.c | 2 ++
> tests/utils.c | 22 ++++++++++++++--------
> utils.c | 6 ++----
> utils.h | 2 ++
> 5 files changed, 28 insertions(+), 17 deletions(-)
>
> diff --git a/tests/main.c b/tests/main.c
> index e1a8b69..1344bc0 100644
> --- a/tests/main.c
> +++ b/tests/main.c
> @@ -38,14 +38,14 @@ static SuiteFunction *suites[] = {
> NULL,
> };
>
> +extern void set_opts_dir(char *);
> +
> static inline void
> print_usage(FILE *stream, char *progname)
> {
> fprintf(stream, "Usage: %s <-d directory>\n", progname);
> }
>
> -char *opts_directory;
> -
> int
> main(int argc, char *argv[])
> {
> @@ -54,12 +54,12 @@ main(int argc, char *argv[])
> int number_failed;
> SuiteFunction *sf;
>
> - opts_directory = NULL;
> + char *opts_dir = NULL;
>
> while ((opt = getopt(argc, argv, "d:t:h")) != -1) {
> switch (opt) {
> case 'd':
> - opts_directory = strdup(optarg);
> + opts_dir = strdup(optarg);
> break;
> case 'h':
> /* fall though !! */
> @@ -69,10 +69,11 @@ main(int argc, char *argv[])
> }
> }
>
> - if (opts_directory == NULL) {
> + if (opts_dir == NULL) {
> print_usage(stdout, argv[0]);
> exit(1);
> }
> + set_opts_dir(opts_dir);
>
> i = 0;
> number_failed = 0;
> @@ -88,6 +89,8 @@ main(int argc, char *argv[])
> i++;
> sf = suites[i];
> }
> + set_opts_dir(NULL);
> + free(opts_dir);
>
> return number_failed;
> }
> diff --git a/tests/ptest_list.c b/tests/ptest_list.c
> index e0ec276..081f027 100644
> --- a/tests/ptest_list.c
> +++ b/tests/ptest_list.c
> @@ -29,6 +29,8 @@
>
> #include "ptest_list.h"
>
> +extern Suite *ptest_list_suite(void);
> +
> static int ptests_num = 6;
> static char *ptest_names[] = {
> "python",
> diff --git a/tests/utils.c b/tests/utils.c
> index 571d488..4fa4609 100644
> --- a/tests/utils.c
> +++ b/tests/utils.c
> @@ -32,9 +32,15 @@
> #include "ptest_list.h"
> #include "utils.h"
>
> +Suite *utils_suite(void);
> +
> #define PRINT_PTEST_BUF_SIZE 8192
>
> -extern char *opts_directory;
> +static char *opts_directory = NULL;
> +
> +void set_opts_dir(char * od) {
> + opts_directory = od;
> +}
>
> static char *ptests_found[] = {
> "bash",
> @@ -68,7 +74,7 @@ find_word(int *found, const char *line, const char *word)
> }
>
> static void test_ptest_expected_failure(struct ptest_list *, const int,
> char *,
> - void (*h_analizer)(const int, FILE *, FILE *));
> + void (*h_analyzer)(const int, FILE *));
>
> START_TEST(test_get_available_ptests)
> {
> @@ -189,7 +195,7 @@ START_TEST(test_run_ptests)
> END_TEST
>
> static void
> -search_for_timeout_and_duration(const int rp, FILE *fp_stdout, FILE
> *fp_stderr)
> +search_for_timeout_and_duration(const int rp, FILE *fp_stdout)
> {
> const char *timeout_str = "TIMEOUT";
> const char *duration_str = "DURATION";
> @@ -218,7 +224,7 @@ START_TEST(test_run_timeout_duration_ptest)
> END_TEST
>
> static void
> -search_for_fail(const int rp, FILE *fp_stdout, FILE *fp_stderr)
> +search_for_fail(const int rp, FILE *fp_stdout)
> {
> const char *fail_str = "ERROR: Exit status is";
> char line_buf[PRINT_PTEST_BUF_SIZE];
> @@ -284,7 +290,7 @@ START_TEST(test_xml_fail)
> END_TEST
>
> Suite *
> -utils_suite()
> +utils_suite(void)
> {
> Suite *s;
> TCase *tc_core;
> @@ -308,7 +314,7 @@ utils_suite()
>
> static void
> test_ptest_expected_failure(struct ptest_list *head, const int timeout,
> char *progname,
> - void (*h_analizer)(const int, FILE *, FILE *))
> + void (*h_analyzer)(const int, FILE *))
> {
> char *buf_stdout;
> size_t size_stdout = PRINT_PTEST_BUF_SIZE;
> @@ -329,9 +335,9 @@ test_ptest_expected_failure(struct ptest_list *head,
> const int timeout, char *pr
> struct ptest_options opts = EmptyOpts;
> opts.timeout = timeout;
>
> - h_analizer(
> + h_analyzer(
> run_ptests(filtered, opts, progname, fp_stdout,
> fp_stderr),
> - fp_stdout, fp_stderr
> + fp_stdout
> );
>
> PTEST_LIST_FREE_ALL_CLEAN(filtered);
> diff --git a/utils.c b/utils.c
> index 92654bf..a8ba190 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -288,8 +288,7 @@ run_child(char *run_ptest, int fd_stdout, int
> fd_stderr)
> }
>
> static inline int
> -wait_child(const char *ptest_dir, const char *run_ptest, pid_t pid,
> - int timeout, int *fds, FILE **fps, int *timeouted)
> +wait_child(pid_t pid, int timeout, int *fds, FILE **fps, int *timeouted)
> {
> struct pollfd pfds[2];
> struct timespec sentinel;
> @@ -490,8 +489,7 @@ run_ptests(struct ptest_list *head, const struct
> ptest_options opts,
> fprintf(fp, "%s\n", get_stime(stime,
> GET_STIME_BUF_SIZE, sttime));
> fprintf(fp, "BEGIN: %s\n", ptest_dir);
>
> - status = wait_child(ptest_dir,
> p->run_ptest, child,
> - opts.timeout, fds, fps,
> &timeouted);
> + status = wait_child(child, opts.timeout,
> fds, fps, &timeouted);
> entime = time(NULL);
> duration = entime - sttime;
>
> diff --git a/utils.h b/utils.h
> index f6a56f1..aa53707 100644
> --- a/utils.h
> +++ b/utils.h
> @@ -53,4 +53,6 @@ extern FILE *xml_create(int, char *);
> extern void xml_add_case(FILE *, int, const char *, int, int);
> extern void xml_finish(FILE *);
>
> +void set_opts_dir(char * od);
> +
> #endif
> --
> 2.17.0
>
>
[-- Attachment #2: Type: text/html, Size: 8619 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [ptest-runner][PATCH 4/4] Fix additional warnings when using clang
2019-08-01 18:19 ` Anibal Limon
@ 2019-08-01 18:34 ` Randy MacLeod
0 siblings, 0 replies; 7+ messages in thread
From: Randy MacLeod @ 2019-08-01 18:34 UTC (permalink / raw)
To: Anibal Limon, Richard Purdie; +Cc: yocto
On 8/1/19 2:19 PM, Anibal Limon wrote:
> Hi Randy,
>
> Just push your clang fixes to master and created v2.3.2 tag, feel free
> to send a recipe upgrade.
>
> Regards,
> Anibal
Hi Anibal,
Excellent! I will send a recipe update.
Thanks,
--
# Randy MacLeod
# Wind River Linux
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [ptest-runner][PATCH 1/4] utils: ensure child can be session leader
2019-07-17 18:35 [ptest-runner][PATCH 1/4] utils: ensure child can be session leader Randy MacLeod
` (2 preceding siblings ...)
2019-07-17 18:35 ` [ptest-runner][PATCH 4/4] Fix additional warnings when using clang Randy MacLeod
@ 2019-07-21 19:22 ` Anibal Limon
3 siblings, 0 replies; 7+ messages in thread
From: Anibal Limon @ 2019-07-21 19:22 UTC (permalink / raw)
To: Randy MacLeod; +Cc: yocto
[-- Attachment #1: Type: text/plain, Size: 8273 bytes --]
Hi Randy,
Thanks for the patches, I pushed this one to master, I will review the
clang warning patches.
Cheers,
Anibal
On Wed, 17 Jul 2019 at 13:35, Randy MacLeod <Randy.MacLeod@windriver.com>
wrote:
> When running the run-execscript bash ptest as a user rather than root, a
> warning:
> bash: cannot set terminal process group (16036): Inappropriate ioctl for
> device
> bash: no job control in this shell
> contaminates the bash log files causing the test to fail. This happens only
> when run under ptest-runner and not when interactively testing!
>
> The changes made to fix this include:
> 1. Get the process group id (pgid) before forking,
> 2. Set the pgid in both the parent and child to avoid a race,
> 3. Find, open and set permission on the child tty, and
> 4. Allow the child to attach to controlling tty.
>
> Also add '-lutil' to Makefile. This lib is from libc and provides openpty.
>
> Upstream-Status: Submitted [yocto@yoctoproject.org]
>
> Signed-off-by: Sakib Sajal <sakib.sajal@windriver.com>
> Signed-off-by: Randy MacLeod <Randy.MacLeod@windriver.com>
> ---
> Makefile | 2 +-
> utils.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 92 insertions(+), 12 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 1bde7be..439eb79 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -29,7 +29,7 @@ TEST_DATA=$(shell echo `pwd`/tests/data)
> all: $(SOURCES) $(EXECUTABLE)
>
> $(EXECUTABLE): $(OBJECTS)
> - $(CC) $(LDFLAGS) $(OBJECTS) -o $@
> + $(CC) $(LDFLAGS) $(OBJECTS) -lutil -o $@
>
> tests: $(TEST_SOURCES) $(TEST_EXECUTABLE)
>
> diff --git a/utils.c b/utils.c
> index ad737c2..f11ce39 100644
> --- a/utils.c
> +++ b/utils.c
> @@ -1,5 +1,6 @@
> /**
> * Copyright (c) 2016 Intel Corporation
> + * Copyright (C) 2019 Wind River Systems, Inc.
> *
> * This program is free software; you can redistribute it and/or
> * modify it under the terms of the GNU General Public License
> @@ -22,23 +23,27 @@
> */
>
> #define _GNU_SOURCE
> +
> #include <stdio.h>
>
> +#include <dirent.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <grp.h>
> #include <libgen.h>
> -#include <signal.h>
> #include <poll.h>
> -#include <fcntl.h>
> +#include <pty.h>
> +#include <signal.h>
> +#include <stdlib.h>
> +#include <string.h>
> #include <time.h>
> -#include <dirent.h>
> +#include <unistd.h>
> +
> +#include <sys/ioctl.h>
> #include <sys/resource.h>
> +#include <sys/stat.h>
> #include <sys/types.h>
> #include <sys/wait.h>
> -#include <sys/stat.h>
> -#include <unistd.h>
> -#include <string.h>
> -#include <stdlib.h>
> -
> -#include <errno.h>
>
> #include "ptest_list.h"
> #include "utils.h"
> @@ -346,6 +351,53 @@ wait_child(const char *ptest_dir, const char
> *run_ptest, pid_t pid,
> return status;
> }
>
> +/* Returns an integer file descriptor.
> + * If it returns < 0, an error has occurred.
> + * Otherwise, it has returned the slave pty file descriptor.
> + * fp should be writable, likely stdout/err.
> + */
> +static int
> +setup_slave_pty(FILE *fp) {
> + int pty_master = -1;
> + int pty_slave = -1;
> + char pty_name[256];
> + struct group *gptr;
> + gid_t gid;
> + int slave = -1;
> +
> + if (openpty(&pty_master, &pty_slave, pty_name, NULL, NULL) < 0) {
> + fprintf(fp, "ERROR: openpty() failed with: %s.\n",
> strerror(errno));
> + return -1;
> + }
> +
> + if ((gptr = getgrnam(pty_name)) != 0) {
> + gid = gptr->gr_gid;
> + } else {
> + /* If the tty group does not exist, don't change the
> + * group on the slave pty, only the owner
> + */
> + gid = -1;
> + }
> +
> + /* chown/chmod the corresponding pty, if possible.
> + * This will only work if the process has root permissions.
> + */
> + if (chown(pty_name, getuid(), gid) != 0) {
> + fprintf(fp, "ERROR; chown() failed with: %s.\n",
> strerror(errno));
> + }
> +
> + /* Makes the slave read/writeable for the user. */
> + if (chmod(pty_name, S_IRUSR|S_IWUSR) != 0) {
> + fprintf(fp, "ERROR: chmod() failed with: %s.\n",
> strerror(errno));
> + }
> +
> + if ((slave = open(pty_name, O_RDWR)) == -1) {
> + fprintf(fp, "ERROR: open() failed with: %s.\n",
> strerror(errno));
> + }
> + return (slave);
> +}
> +
> +
> int
> run_ptests(struct ptest_list *head, const struct ptest_options opts,
> const char *progname, FILE *fp, FILE *fp_stderr)
> @@ -362,6 +414,8 @@ run_ptests(struct ptest_list *head, const struct
> ptest_options opts,
> int timeouted;
> time_t sttime, entime;
> int duration;
> + int slave;
> + int pgid = -1;
>
> if (opts.xml_filename) {
> xh = xml_create(ptest_list_length(head),
> opts.xml_filename);
> @@ -379,7 +433,6 @@ run_ptests(struct ptest_list *head, const struct
> ptest_options opts,
> close(pipefd_stdout[1]);
> break;
> }
> -
> fprintf(fp, "START: %s\n", progname);
> PTEST_LIST_ITERATE_START(head, p);
> char *ptest_dir = strdup(p->run_ptest);
> @@ -388,6 +441,13 @@ run_ptests(struct ptest_list *head, const struct
> ptest_options opts,
> break;
> }
> dirname(ptest_dir);
> + if (ioctl(0, TIOCNOTTY) == -1) {
> + fprintf(fp, "ERROR: Unable to detach from
> controlling tty, %s\n", strerror(errno));
> + }
> +
> + if ((pgid = getpgid(0)) == -1) {
> + fprintf(fp, "ERROR: getpgid() failed,
> %s\n", strerror(errno));
> + }
>
> child = fork();
> if (child == -1) {
> @@ -395,13 +455,33 @@ run_ptests(struct ptest_list *head, const struct
> ptest_options opts,
> rc = -1;
> break;
> } else if (child == 0) {
> - setsid();
> + close(0);
> + if ((slave = setup_slave_pty(fp)) < 0) {
> + fprintf(fp, "ERROR: could not
> setup pty (%d).", slave);
> + }
> + if (setpgid(0,pgid) == -1) {
> + fprintf(fp, "ERROR: setpgid()
> failed, %s\n", strerror(errno));
> + }
> +
> + if (setsid() == -1) {
> + fprintf(fp, "ERROR: setsid()
> failed, %s\n", strerror(errno));
> + }
> +
> + if (ioctl(0, TIOCSCTTY, NULL) == -1) {
> + fprintf(fp, "ERROR: Unable to
> attach to controlling tty, %s\n", strerror(errno));
> + }
> +
> run_child(p->run_ptest, pipefd_stdout[1],
> pipefd_stderr[1]);
> +
> } else {
> int status;
> int fds[2]; fds[0] = pipefd_stdout[0];
> fds[1] = pipefd_stderr[0];
> FILE *fps[2]; fps[0] = fp; fps[1] =
> fp_stderr;
>
> + if (setpgid(child, pgid) == -1) {
> + fprintf(fp, "ERROR: setpgid()
> failed, %s\n", strerror(errno));
> + }
> +
> sttime = time(NULL);
> fprintf(fp, "%s\n", get_stime(stime,
> GET_STIME_BUF_SIZE, sttime));
> fprintf(fp, "BEGIN: %s\n", ptest_dir);
> --
> 2.17.0
>
>
[-- Attachment #2: Type: text/html, Size: 10437 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread