Opened 5 years ago
Closed 5 years ago
#13409 closed Patch - Bug Fix (fixed)
Valgrind error in dvbci.cpp
Reported by: | Klaas de Waal | Owned by: | Klaas de Waal |
---|---|---|---|
Priority: | minor | Milestone: | 31.0 |
Component: | MythTV - DVB | Version: | Master Head |
Severity: | low | Keywords: | DVBCI CAM |
Cc: | Stuart Auchterlonie, gigem | Ticket locked: | no |
Description
Running mythbackend with valgrind gives the following error message:
==16878== Thread 28 DVBCam: ==16878== Syscall param write(buf) points to uninitialised byte(s) ==16878== at 0x8BB1D57: __libc_write (write.c:26) ==16878== by 0x8BB1D57: write (write.c:24) ==16878== by 0x6D32D72: cTPDU::Write(int) (dvbci.cpp:333) ==16878== by 0x6D33909: cCiTransportConnection::SendTPDU(unsigned char, int, unsigned char const*) (dvbci.cpp:454) ==16878== by 0x6D33DB4: cCiTransportConnection::SendData(int, unsigned char const*) (dvbci.cpp:525) ==16878== by 0x6D353D3: cCiSession::SendData(int, int, unsigned char const*) (dvbci.cpp:858) ==16878== by 0x6D38850: cCiDateTime::SendDateTime() (dvbci.cpp:1160) ==16878== by 0x6D38ABE: cCiDateTime::Process(int, unsigned char const*) (dvbci.cpp:1180) ==16878== by 0x6D3CC9C: cLlCiHandler::Process() (dvbci.cpp:1780) ==16878== by 0x6D21913: DVBCam::run() (dvbcam.cpp:268) ==16878== by 0x71AD306: MThread::run() (mthread.cpp:322) ==16878== by 0x71ADC7F: MThreadInternal::run() (mthread.cpp:79) ==16878== by 0x9D332FA: QThreadPrivate::start(void*) (qthread_unix.cpp:367) ==16878== Address 0x36eda7f6 is on thread 28's stack ==16878== in frame #2, created by cCiTransportConnection::SendTPDU(unsigned char, int, unsigned char const*) (dvbci.cpp:452) ==16878== Uninitialised value was created by a stack allocation ==16878== at 0x6D38473: cCiDateTime::SendDateTime() (dvbci.cpp:1133) ==16878==
This is caused by code in SendDateTime? that composes a 7-byte string with date and time in the format for the CAM.
The original code is this:
struct tTime { unsigned short mjd; uint8_t h, m, s; short offset; }; tTime T; T.mjd = htons(MJD); T.h = DEC2BCD(tm_gmt.tm_hour); T.m = DEC2BCD(tm_gmt.tm_min); T.s = DEC2BCD(tm_gmt.tm_sec); T.offset = static_cast<short>(htons(tm_loc.tm_gmtoff / 60)); ... SendData(AOT_DATE_TIME, 7, (uint8_t*)&T);
The struct is filled with the data and then the address of the struct is passed on as a pointer to a byte array.
The error is that the struct is 8 bytes long and not 7 bytes; there is a one byte hole between T.s and T.offset.
The result is that:
- the hole between T.s and T.offset causes the Valgrind error message
- the first byte of T.offset is sent on the position of the second byte
- the second byte of T.offset is not sent
This can be fixed by using a byte array and copying the values to the correct position in the byte array, as shown here:
#define BYTE0(a) ((a) & 0xFF) #define BYTE1(a) (((a) >> 8) & 0xFF) uint8_t T[7]; uint16_t mjd = htons(MJD); int16_t local_offset = htons(tm_loc.tm_gmtoff / 60); T[0] = BYTE0(mjd); T[1] = BYTE1(mjd); T[2] = DEC2BCD(tm_gmt.tm_hour); T[3] = DEC2BCD(tm_gmt.tm_min); T[4] = DEC2BCD(tm_gmt.tm_sec); T[5] = BYTE0(local_offset); T[6] = BYTE1(local_offset);
This is a message sent to the CAM with the original code:
2019-02-19 16:34:47.294282 I --> 00 01 a0 10 01 90 02 00 04 9f 84 41 07 e4 a5 15 34 47 00 00
This is a message sent to the CAM with the fix applied:
2019-02-19 17:06:09.019635 I --> 00 01 a0 10 01 90 02 00 04 9f 84 41 07 e4 a5 16 06 09 00 3c
In this message, the last two bytes is the offset from local time to the UTC time in minutes; the three bytes before that is the UTC time.
With the fix the time is correct, as can be verified by comparing the log timestamp 17:06:09.019635 with the UTC time 16:06:09 and the offset of 60 minutes.
The patch is attached.
Attachments (2)
Change History (7)
Changed 5 years ago by
Attachment: | 20190219-dvbci-datetime.patch added |
---|
comment:1 Changed 5 years ago by
Wouldn't it be easier to just attached the 'packed' attribute to the structure?
comment:2 Changed 5 years ago by
Yes, using the packed attribute would also work. However, attributes are implementation/compiler dependent. Also there is memory alignment; in this case a 16-bit value is accessed at an odd address if the struct is packed.
Copying the bytes to the exact location of a byte array always works independent of the compiler, the compiler flags, attributes and underlying hardware architecture.
comment:3 Changed 5 years ago by
The typecast in SendData?, to convert a pointer to the struct to a pointer to a byte array, is not needed when a byte array is used.
Attached is a new version of the patch in which also the typecast is removed.
Changed 5 years ago by
Attachment: | 20190220-dvbci-datetime.patch added |
---|
comment:4 Changed 5 years ago by
Milestone: | needs_triage → 31.0 |
---|---|
Owner: | set to Klaas de Waal |
Status: | new → accepted |
Fix offset of local time to UTC in DateTime? string to CAM