[genivi-dlt] Some patches for DLT Daemon, DLT System

Lipka, Christoph (ADITJ/SWG) clipka at jp.adit-jv.com
Mon Jan 16 19:33:21 EST 2017


Hi Stefan,

1. looks good.
2. introduces tabs. I prefer to have 4spaces instead of tabs in the code. The indentation is not correct as well:

vi src/lib/dlt_client.c +522
523     else$
524     {$
525 >---uint total_size = msg.headersize-sizeof(DltStorageHeader)+msg.datasize;$
526 >---char buf[total_size];$
527 >---memset(buf,0,total_size);$
528 >---memcpy(buf,msg.headerbuffer+sizeof(DltStorageHeader),msg.headersize-sizeof(DltStorageHeader));$
529 >---memcpy(buf+msg.headersize-sizeof(DltStorageHeader),msg.databuffer,msg.datasize);$
530 >---send(client->sock, (const char *)buf,total_size,0);$
531     }$

What is the problem to send 2 messages? Copying both into one buffer before looks like a big overhead to me.

I also prefer a whitespace after comma, before/after “-“ and so on for readability. I know, daemon code has no coding rules, good coding style, but why we should not improve here.

3. and 4.
Both patches have wrong user: user <user at sb.gl<mailto:user at sb.gl>> (Mon 02 Jan 2017 05:32:04 PM JST), before you have used: Stefan Badura <stefan.badura at globallogic.com<mailto:stefan.badura at globallogic.com>> (Mon 02 Jan 2017 04:48:49 PM JST)

For patch 3: Could you please explain what problem you trying to solve? The commit message is not clear enough. And a commit for dlt master should not contain “hox fix” I think. Is it really necessary to allocate and memcpy header and payload into a single buffer before sending? Having in mind that there might be hundreds or thousands of messages per seconds it might slow down the daemon.

For patch 4: You mix tabs and spaces for whitespace (still prefer tabs). The daemon contains too many too long lines. I prefer to not introduce new:

87             DLT_LOG(dltsystem, DLT_LOG_ERROR, DLT_STRING("Dlt System: Connection to the DLT not established ye    t! Trying again in a second ..."));$

Preferred:
                   DLT_LOG(dltsystem,
DLT_LOG_ERROR,
DLT_CSTRING("Dlt System: Connection to the DLT not established yet! Trying again in a second ..."));$ // You may use a constant string instead.


Might it be possible to work on the findings? Is it possible for you to add patches as pull request in github? This makes review and integration easier.

Thanks.

Best regards

Christoph Lipka
Software Group (ADITJ/SWG)

Tel. +81-(0)566 61-5124

From: genivi-diagnostic-log-and-trace [mailto:genivi-diagnostic-log-and-trace-bounces at mailman1.genivi.org]<mailto:[mailto:genivi-diagnostic-log-and-trace-bounces at mailman1.genivi.org]> On Behalf Of Stefan Badura via genivi-diagnostic-log-and-trace
Sent: Friday, January 13, 2017 10:24 PM
To: genivi-diagnostic-log-and-trace at lists.genivi.org<mailto:genivi-diagnostic-log-and-trace at lists.genivi.org>
Cc: Rastislav Krivy
Subject: [genivi-dlt] Some patches for DLT Daemon, DLT System

Hi all,

We would like to propose some patches (fixies) for dlt, which have solved some several rarely occurred issues in dlt-daemon and dlt-system.
We hope it can be helpful for someone.

1. Value initialization in user library: 001_values_initialization.patch.
2. Patch for sending messages to dlt client via socket  - sending message in parts (header, payload) caused troubles. Is there any special idea behind sending messages by parts? Sending message in single step solved the problem. See 002_sending_messages_via_socket.patch.
3. Dlt-daemon sending to socket - In current implementation, corrupted messages occur rarely, when sending data into socket. The hotfix is proposed - creating temporal buffer. See 003_corrupted_messages.patch.
4. Dlt-system logging problem - time to time dlt-system was not able to log into dlt-daemon. The problem  was fixed by providing active waiting in dlt-system until dlt-user library is initialized. See 004_waiting_for_init.patch.
With best regards
Stefan Badura | Software Engineer
GlobalLogic
Vysokoškolákov 1757/1,
Žilina 010 01
Slovakia
www.globallogic.com<http://www.globallogic.com/>
http://www.globallogic.com/email_disclaimer.txt

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.genivi.org/pipermail/genivi-diagnostic-log-and-trace_lists.genivi.org/attachments/20170117/af4678d7/attachment.html>


More information about the genivi-diagnostic-log-and-trace mailing list