On 24.11.17 01:56, Juha Heinanen wrote:
When adding transport proto check to
add_contact_alias, I noticed that
in receive_info struct proto is defined like this:
char proto;
and in sip_uri struct like this:
unsigned short proto; /*!< from transport */
That does not look good to me. They should have the same type in both.
I also noticed a mismatch on the type of the same field in different
structures, in some cases it seems it was done to have the fields
properly aligned in memory, otherwise accessing it via &struct.field can
give a SIGBUS on some architectures.
I am fine to get such fields to the same type, just be careful to have
them aligned to 32bits from the start of the structure.
Somehow I got to the conclusion that most of the
shorter-than-sizeof(int) fields should be converted to int, there are
not too many and the overall size increase is not relevant, being safer
on avoiding a sigbus when one adds a new field. Quite often is hard to
predict the impact of adding a non 32b or 64b field in a structure,
because that structure can be used for fields in other structures, so
even not accessing that field by its address can mess up the access of
other fields from other structures.
In some cases I noticed that a char or short int is also used to store
values which are defined in an enum, on the other hand an enum field is
stored as integer, so again a mismatch of size on propagating values
from fields with same purpose -- these I tried to fix in the past, as
some code analyzers could detect them, I am giving it as an example that
having at least the int size is a good/safe option.
Back this specific case -- the proto -- I expect there are many other
structures having it. It might be good to define a type for it and then
change as discovered -- for example:
typedef uint32_t sr_proto_t;
Cheers,
Daniel
--
Daniel-Constantin Mierla
www.twitter.com/miconda --
www.linkedin.com/in/miconda
Kamailio Advanced Training -
www.asipto.com
Kamailio World Conference - May 14-16, 2018 -
www.kamailioworld.com