Underscore scoping and taint

[Ketho]

Someone provided me with a patch for the addon HideRaidFrame, saying that it would fix the taint

This is the full code (HideRaidFrame.lua)

-------------------------------------------
--- Author: Ketho (EU-Boulderfist)      ---
--- License: Public Domain              ---
--- Created: 2011.07.06                 ---
--- Version: 0.9 [2012.08.28]           ---
-------------------------------------------
--- Curse           [url]http://www.curse.com/addons/wow/hideraidframe[/url]
--- WoWInterface    [url]http://www.wowinterface.com/downloads/info20052-HideRaidFrame.html[/url]
 
local NAME, S = ...
local VERSION = 0.9
local BUILD = "Release"
 
local ACR = LibStub("AceConfigRegistry-3.0")
local ACD = LibStub("AceConfigDialog-3.0")
 
local L = S.L
 
local db
local pendingReload
 
local function ToggleAddOn(v)
    local f = (v or not db.HardDisable) and EnableAddOn or DisableAddOn
    f("Blizzard_CompactRaidFrames")
    f("Blizzard_CUFProfiles")
end
 
local frames = {"Manager", "Container"}
 
    ---------------
    --- Options ---
    ---------------
 
local options = {
    type = "group",
    name = format("%s |cffADFF2Fv%s|r", NAME, VERSION),
    get = function(i)
        return db[i[#i]]
    end,
    set = function(i, v)
        db[i[#i]] = v
        if db.Manager or db.Container then
            db.HardDisable = false
        end
        pendingReload = true
        ToggleAddOn(db.Manager or db.Container)
    end,
    args = {
        inline1 = {
            type = "group", order = 1,
            name = " ",
            inline = true,
            args = {
                Manager = {
                    type = "toggle", order = 1,
                    width = "full", descStyle = "",
                    name = " "..L.RAID_MANAGER,
                },
                Container = {
                    type = "toggle", order = 2,
                    width = "full", descStyle = "",
                    name = " "..L.RAID_CONTAINER,
                },
            },
        },
        HardDisable = {
            type = "toggle", order = 2,
            desc = L.HARD_DISABLE_DESC,
            name = L.HARD_DISABLE,
            disabled = function() return db.Manager or db.Container end,
        },
        Reload = {
            type = "execute", order = 3,
            descStyle = "",
            name = "|TInterface\\OptionsFrame\\UI-OptionsFrame-NewFeatureIcon:0:0:-2|t"..SLASH_RELOAD1,
            func = ReloadUI,
            hidden = function() return not pendingReload end,
        },
    },
}
 
    ----------------------
    --- Initialization ---
    ----------------------
 
local f = CreateFrame("Frame")
 
function f:OnEvent(event, addon)
    if addon ~= NAME then return end
    
    HideRaidFrameDB3 = HideRaidFrameDB3 or {}
    db = HideRaidFrameDB3
    db.version = VERSION
    
    ACR:RegisterOptionsTable(NAME, options)
    ACD:AddToBlizOptions(NAME, NAME)
    ACD:SetDefaultSize(NAME, 380, 200)
    
    if IsAddOnLoaded("Blizzard_CompactRaidFrames") then
        
        -- InCombatLockdown does not readily seem to return the correct value though
        if InCombatLockdown() then print(format("|cff33FF99%s:|r %s", NAME, ERR_NOT_IN_COMBAT)) return end
        
        -- CompactRaidFrameManager is parented to UIParent
        -- CompactRaidFrameContainer is parented to CompactRaidFrameManager
        -- bug: Container (if enabled) will still be shown when solo, after leaving a raid
        CompactRaidFrameContainer:SetParent(UIParent)
        
        for _, v in ipairs(frames) do
            if not db[v] then
                local f = _G["CompactRaidFrame"..v]
                f:UnregisterAllEvents()
                f.Show = function() end
                f:Hide()
            end
        end
    end
    
    ToggleAddOn(db.Manager or db.Container)
    self:UnregisterEvent("ADDON_LOADED")
end
 
f:RegisterEvent("ADDON_LOADED")
f:SetScript("OnEvent", f.OnEvent)
 
    ---------------------
    --- Slash Command ---
    ---------------------
 
local slashCmds = {"hr", "hrf", "hideraid", "hideraidframe"}
 
for i, v in ipairs(slashCmds) do
    _G["SLASH_HIDERAIDFRAME"..i] = "/"..v
end
 
SlashCmdList.HIDERAIDFRAME = function(msg, editbox)
    ACD:Open(NAME)
end

And this is the proposed patch (line 109)

		for null, v in ipairs(frames) do
			if not db[v] then
				local f = _G["CompactRaidFrame"..v]
				f:UnregisterAllEvents()
				f.Show = function() end
				f:Hide()
			end
		end

The thing is, I don't really think it makes any difference since the underscore should be in scope to the ipairs-loop

Could anyone say if this would fix any taint?

[Dridzt]

That's correct (what you said).
The _ is local to the ipairs loop in this case.

[Dridzt]

You can try hardening your incombat check a little, because InCombatLockdown() won't return proper value if you log/relog in combat.

Best you check with:
InCombatLockdown() or UnitAffectingCombat("player") or UnitAffectingCombat("pet")

[Barjack]

I always assumed InCombatLockdown() returning false when you log in during combat was intentional so as to provide a small window of opportunity to setup secure frames even while actually in combat. Is that not the case?

Regarding the use of _, my understanding is that any variables in that position will automatically be shadowed, so I wouldn't expect any difference. It may not hurt to try it just to see though, since I'm not sure how Blizzard's taint system works under the hood.

[Torhal]

The patch is inconsequential. Variables declared in a loop are local to the loop, so the underscore never references the global version.