Re: [Hamlib-developer] RFC #807--Change rig ptr checks to function to check valid rig ptr in list

[Daniele F. <iu...@gm...>]

George Baltz wrote:

> I think the first half of this is done - rig_open() builds a list of
> opened rig_structs, but nobody checks it on each rig_*() call. Overhead
> problem?

maby, but there is already some overhead because many (all?) functions
use the macro
#define CHECK_RIG_ARG(r) (!(r) || !(r)->caps || !STATE((r))->comm_state)

in some cases the last macro check is equivalent to checking if the
rig is in the list because rig_close() calls remove_opened_rig(rig);
and sets rs->comm_state = 0;
So comm_state is certainly zero when the struct is not in the list (it
may be zero even when the rig is in the list but it is a different
case).

rig_close is like this:

int HAMLIB_API rig_close(RIG *rig)
{
[...]
    if (!rs->comm_state)
    {
        RETURNFUNC(-RIG_EINVAL);
    }

    remove_opened_rig(rig);
[...]
    rs->comm_state = 0;
[...]
}


However issue #785 talked about memory corruption (so that even
comm_state may appear non-zero when the rig is no more in the list and
what I have written above doesn't hold).
I'm not sure if this issue #807 still covers the memory corruption,
but I don't think that protecting against this kind of memory
corruption is worth our time when there are many fixed length buffers
and suspicious uses of sprintf-like functions.

This list of open rigs could be useful if rigctld gets the ability to
handle more than one rig at a time, but it would add extra complexity
to clients, that in some cases can be avoided running more copies of
rigctld on different ports.

If everybody agrees that it is a rare case not worth protecting
against, that memory corruption affects only a rig pointer and not
other important data or the stack, can we close this?

-- 
73 de IU5HKX Daniele