[Beignet] [PATCH] Use syslog for device self-test errors

[Giuseppe Bilotta]

On Mon, Oct 12, 2015 at 3:43 AM, Yang, Rong R <rong.r.yang at intel.com> 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:beignet-bounces at lists.freedesktop.org] On Behalf Of
>> Giuseppe Bilotta
>> Sent: Sunday, September 27, 2015 19:47
>> To: beignet at lists.freedesktop.org
>> 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
>> Beignet at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/beignet



-- 
Giuseppe "Oblomov" Bilotta