[Live-devel] (no subject)

[Andy Hawkins]

Hi,

Apologies. I didn't read the code as thoroughly as I thought I had. I agree that modifying the code just to shut up Valgrind isn't necessarily a good idea. After re-reading the methods mentioned in Valgrind's stack trace, I agree that the structure should be initialised (by a call to recvfrom in 'GroupsockHelper::readSocket') before anything ever checks that structure.

Apologies for the noise, and I in no way wanted to cast aspersions on the code. I was merely trying to point out an issue that I mistakenly thought Valgrind had identified.

Thanks

Andy


From: live-devel <live-devel-bounces at us.live555.com> on behalf of Ross Finlayson <finlayson at live555.com>
Sent: 30 November 2023 15:45
To: LIVE555 Streaming Media - development & use <live-devel at us.live555.com>
Subject: Re: [Live-devel] (no subject)

I think ‘valgrind’ is mistaken/confused here, because “sockaddr_storage” is a structure whose contents depend on the address type being stored.  E.g., on my FreeBSD system, it’s defined as follows:

#define _SS_MAXSIZE     128U
#define _SS_ALIGNSIZE   (sizeof(__int64_t))
#define _SS_PAD1SIZE    (_SS_ALIGNSIZE - sizeof(unsigned char) - \
                            sizeof(sa_family_t))
#define _SS_PAD2SIZE    (_SS_MAXSIZE - sizeof(unsigned char) - \
                            sizeof(sa_family_t) - _SS_PAD1SIZE - _SS_ALIGNSIZE)

struct sockaddr_storage {
        unsigned char   ss_len;         /* address length */
        sa_family_t     ss_family;      /* address family */
        char            __ss_pad1[_SS_PAD1SIZE];
        __int64_t       __ss_align;     /* force desired struct alignment */
        char            __ss_pad2[_SS_PAD2SIZE];
};

I.e., not all bytes in this structure will be used - and that’s OK, as long as no part of the structure is ever read without having been previously written to.  And that should be the case in our code; in particular, in "MultiFramedRTPSource::networkReadHandler1()”, “fromAddress” should never be accessed without first having been assigned[*] - even though some *bytes* within the “fromAddress” structure might not actually have been written to.

I’m not a great fan of ‘valgrind’, because people often use it indiscriminately, and post - to this mailing list - what they think are ‘bugs’, without really understanding the true nature of the bug, or if it really is actually a bug.

[*]If parts of our code really do depend upon uninitialized values, then that really would be a bug, and I’ll fix it.  But I’m not going to paper over this my making a change just to keep ‘valgrind’ quiet.


Ross Finlayson
Live Networks, Inc.
http://www.live555.com/


_______________________________________________
live-devel mailing list
live-devel at lists.live555.com
http://lists.live555.com/mailman/listinfo/live-devel