leandromoreira commented 4 years ago

Hi there,

I'm planning to use this library to parse MPEG-Dash manifests, and I ran a lint tool over the source files looking for potential memory leak and others, I couldn't find any but there are some issues I think that we can solve.

I'm gonna list them here and if you're okay, I can make a PR fixing those issues.

XmlParser.lua

Shadowing upvalue

--  XmlParser.lua:131:26: shadowing upvalue err on line 131
local function err(self, err, pos)
    if self.options.errorHandler then
        self.options.errorHandler(err,pos)
    end
end

Suggestion: rename either the function or parameter.

Variable previously defined

--     XmlParser.lua:252:27: variable i was previously defined as a loop variable on line 251
local function _parseDtd(self, xml, pos)
    -- match,endMatch,root,type,name,uri,internal
    local dtdPatterns = {self._DTD1, self._DTD2, self._DTD3, self._DTD4, self._DTD5}

    for i, dtd in pairs(dtdPatterns) do
        local m,e,r,t,n,u,i = string.find(xml, dtd, pos)
        if m then
            return m, e, {_root=r, _type=t, _name=n, _uri=u, _internal=i} 
        end
    end

    return nil
end

Suggestion: rename either the the first i or the second.

Setting non-standard global variable

--     XmlParser.lua:262:26: setting non-standard global variable attrs
local function parseDtd(self, xml, f)
    f.match, f.endMatch, attrs = _parseDtd(self, xml, f.pos)
    if not f.match then 
        err(self, self._errstr.dtdErr, f.pos)
    end 

    if fexists(self.handler, 'dtd') then
        local tag = {name="DOCTYPE", value=string.sub(xml, f.match+10, f.endMatch-1)}
        self.handler:dtd(tag, f.match, f.endMatch)
    end
end

Suggestion: since the attrs is not used we could replace it with _.

xml2lua.lua

Variable was previously defined

--     xml2lua.lua:151:9: variable attrTable was previously defined as an argument on line 149
local function attrToXml(attrTable)
  local s = ""
  local attrTable = attrTable or {}

  for k, v in pairs(attrTable) do
      s = s .. " " .. k .. "=" .. '"' .. v .. '"'
  end
  return s
end

Suggestion: just remove the local keyword.

--    xml2lua.lua:179:9: variable level was previously defined as an argument on line 178
function xml2lua.toXml(tb, tableName, level)
  local level = level or 1
  local firstLevel = level
  local spaces = string.rep(' ', level*2)
  local xmltb = level == 1 and {'<'..tableName..'>'} or {}

Suggestion: just remove the local keyword.

manoelcampos commented 4 years ago

Hey @leandromoreira This is great. If you send a PR, a merge it right away. Thanks for reporting.

manoelcampos commented 4 years ago

I've just fixed issue #45. Please update your fork.