Opened 13 years ago

Closed 13 years ago

#4823 closed defect (fixed)

connections variable is useless

Reported by: Erik Hovland <erik@…> Owned by: Nigel
Priority: minor Milestone: 0.22
Component: mythtv Version: 0.21-fixes
Severity: low Keywords:
Cc: Ticket locked: no


In the member function MythContext::PromptForSchemaUpgrade?() the bool variable 'connections' is useless. So having it there only makes the code more confusing then it should.

I am submitting a patch which comments it out and any code which uses it is changed or removed depending on that change.

This one is a little tricky so forgive the longer description.

First off, connections is never changed by any code. So it is always false. That means that whenever it is used, its evaluation is predetermined. At line 3511, the conditional will always have a true value for the connections test. So the only values that matter are autoUpgrade and upgradable. At line 3518, connections will always evaluate false, so this if can never happen. Even if you remove connections from the conditional, because of the conditional at 3511, this conditional cannot be true. So it is a code path that can never be traversed (even if connections is removed from it). Finally there is some code that uses connections as a conditional that doesn't mean anything. They can safely be removed.

Attachments (1)

libs_libmyth_mythcontext.cpp-connections-is-useless.patch (2.6 KB) - added by Erik Hovland <erik@…> 13 years ago.
removes connections and any code which depends on it

Download all attachments as: .zip

Change History (6)

Changed 13 years ago by Erik Hovland <erik@…>

removes connections and any code which depends on it

comment:1 Changed 13 years ago by Isaac Richards

Milestone: 0.210.22

comment:2 Changed 13 years ago by Nigel

Resolution: invalid
Status: newclosed

That variable isn't useless, it just isn't set yet. Observe line 3488:

    // FIXME: Don't know how to determine this
    //if (getActiveConnections() > 1)
    //    connections = true;

When we work out an efficient way to find the clients that are using a given database, then getActiveConnections() will be written, and this extra variable will be useful. Until then though, I would like to keep it there. It is only one bool that is checked a few times at startup, after all.

comment:3 Changed 13 years ago by Erik Hovland <erik@…>

Resolution: invalid
Status: closednew

Still please consider my patch. It does not actually delete the code, but comment it out. Which means that until you get around to making connections useful you will not be bloating the library. And when you do around to it, the code is ready there for you to uncomment.

comment:4 Changed 13 years ago by danielk

Owner: changed from Isaac Richards to Nigel
Status: newassigned

comment:5 Changed 13 years ago by Nigel

Resolution: fixed
Status: assignedclosed

(In [17748]) When prompting for database schema upgrade or autoupgrading, also check for other myth programs which might be using the database. Closes #4823.

Note: See TracTickets for help on using tickets.