On Mon, Oct 12, 2015 at 3:43 AM, Yang, Rong R <[email protected]> wrote: > It is good idea print warnings to syslog avoid application confuse. > But for errors, I think also need print to stdout, give the clear hint to > application. > > Any other consideration?
In my opinion a library should never produce (console) output unless explicitly requested to do so, so I think using syslog in any case is the correct approach. Maybe have some environment variable (OCL_VERBOSE or something like that) to control output to console? (In any case, even if the printf is preserved, it should at the very least be an fprintf(stderr, ...) instead) >> -----Original Message----- >> From: Beignet [mailto:[email protected]] On Behalf Of >> Giuseppe Bilotta >> Sent: Sunday, September 27, 2015 19:47 >> To: [email protected] >> Cc: Giuseppe Bilotta >> Subject: [Beignet] [PATCH] Use syslog for device self-test errors >> >> --- >> src/cl_device_id.c | 15 +++++++++++---- >> 1 file changed, 11 insertions(+), 4 deletions(-) >> >> diff --git a/src/cl_device_id.c b/src/cl_device_id.c index 78d2cf4..47bc772 >> 100644 >> --- a/src/cl_device_id.c >> +++ b/src/cl_device_id.c >> @@ -36,6 +36,8 @@ >> #include <stdlib.h> >> #include <sys/sysinfo.h> >> >> +#include <syslog.h> >> + >> #ifndef CL_VERSION_1_2 >> #define CL_DEVICE_BUILT_IN_KERNELS 0x103F #endif @@ -611,7 +613,7 >> @@ cl_self_test(cl_device_id device, cl_self_test_res atomic_in_l3_flag) >> ret = SELF_TEST_PASS; >> } else { >> ret = SELF_TEST_SLM_FAIL; >> - printf("Beignet: self-test failed: (3, 7, 5) + (5, 7, >> 3) returned >> (%i, %i, %i)\n" >> + syslog(LOG_ERR, "Beignet: self-test failed: (3, 7, 5) >> + (5, 7, 3) >> returned (%i, %i, %i)\n" >> "See README.md or >> http://www.freedesktop.org/wiki/Software/Beignet/\n";, >> test_data[0], test_data[1], test_data[2]); >> >> @@ -646,6 +648,8 @@ cl_get_device_ids(cl_platform_id platform, >> { >> cl_device_id device; >> >> + openlog("beignet", LOG_CONS, LOG_USER); >> + >> /* Do we have a usable device? */ >> device = cl_get_gt_device(); >> if (device) { >> @@ -653,7 +657,7 @@ cl_get_device_ids(cl_platform_id platform, >> if (ret == SELF_TEST_ATOMIC_FAIL) { >> device->atomic_test_result = ret; >> ret = cl_self_test(device, ret); >> - printf("Beignet: warning - disable atomic in L3 feature.\n"); >> + syslog(LOG_WARNING, "Beignet: Warning - disable atomic in L3 >> + feature.\n"); >> } >> >> if(ret == SELF_TEST_SLM_FAIL) { >> @@ -664,13 +668,16 @@ cl_get_device_ids(cl_platform_id platform, >> sscanf(env, "%i", &disable_self_test); >> } >> if (disable_self_test) { >> - printf("Beignet: Warning - overriding self-test failure\n"); >> + syslog(LOG_WARNING, "Beignet: Warning - overriding self-test >> + failure\n"); >> } else { >> - printf("Beignet: disabling non-working device\n"); >> + syslog(LOG_ERR, "Beignet: disabling non-working device\n"); >> device = 0; >> } >> } >> } >> + >> + closelog(); >> + >> if (!device) { >> if (num_devices) >> *num_devices = 0; >> -- >> 2.6.0.rc2.233.g6dd8a9a.dirty >> >> _______________________________________________ >> Beignet mailing list >> [email protected] >> http://lists.freedesktop.org/mailman/listinfo/beignet -- Giuseppe "Oblomov" Bilotta _______________________________________________ Beignet mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/beignet
