jonnystorm / snmp-elixir

An SNMP client library for Elixir
Mozilla Public License 2.0
33 stars 12 forks source link

Argument error in the SNMP.groom_snmp_result/1 function #52

Closed volnyvolnyvolny closed 4 years ago

volnyvolnyvolny commented 4 years ago

HI! Thanks for the great library!

I have a small problem using it with my MIB. Skipping the unimportant part, the problem is that :snmp returns

pry(2)> result
{:ok,   
 {:noError, 0,
  [
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 12], :INTEGER, 1, 1},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 6], :INTEGER, 29, 2},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 4], :INTEGER, 17, 3},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 10], :INTEGER, 11, 4},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 8], :INTEGER, 29, 5},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 15], :INTEGER, 28, 6},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 2], :INTEGER, 23, 7},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 21], :INTEGER, 5, 8},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 17], :INTEGER, 1, 9},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 20], :INTEGER, 29, 10},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 18], :INTEGER, 1, 11},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 19], :INTEGER, 29, 12},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 16], :INTEGER, 9, 13},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 13], :INTEGER, 28, 14},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 11], :INTEGER, 7, 15},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 9], :INTEGER, 11, 16},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 1], :INTEGER, 2, 17},
    {:varbind, [1, 3, 6, 1, 4, 1, 6827, 10, 316, 5, 1, 1, 14], :INTEGER, 17, 18}
  ]}, 4991}

and then it is parsed with the groom_snmp_result. And in the first clause:

case result do
      {:ok, {:noError, 0, varbinds}, _} ->
        varbinds
        |> Enum.sort_by(sort_fun)
        |> Enum.map(fn {_, oid, type, value, _} ->
          %{oid: oid,
            type: type,
            value: :binary.list_to_bin(value),
          }
        end)

you apply :binary.list_to_bin(value) to the integer value 1.

And that gives me the error:

** (ArgumentError) argument error
    (stdlib) :binary.list_to_bin(1)
    (snmp_ex) lib/snmp.ex:416: anonymous fn/1 in SNMP.groom_snmp_result/1
    (elixir) lib/enum.ex:1336: Enum."-map/2-lists^map/1-0-"/2

Is it necessary to return binary value? Can we add is_list(value) && :binary.list_to_bin(value) || value?

jonnystorm commented 4 years ago

Thanks for contributing!

I believe this was corrected by #49. Would you mind running mix deps.update snmp_ex and recompiling? I expect that will solve the problem, but let me know if you still see errors afterward.

volnyvolnyvolny commented 4 years ago

Indeed! But I don't see 1.3.2 on hex.pm :-)

jonnystorm commented 4 years ago

Oops! I just published version 0.4.0 to Hex.pm. Let me know if that works.

volnyvolnyvolny commented 4 years ago

Yes, thanks. It solves an issue!

By the way, I like the way you use

with v when is_list(v) <- value do
   :binary.list_to_bin(v)
end

In a similar cases I use is_list(v) && :binary.list_to_bin(v) || v or

if is_list(v) do
  :binary.list_to_bin(v)
end || v

but it is ugly.

jonnystorm commented 4 years ago

For what it's worth, the even shorter invocation

with v when is_list(v) <- value,
  do: :binary.list_to_bin(v)

also strikes me as a reasonable substitute where a single guard and a single function call is required.

Thanks, again!