Re: problem with new socket module setopt()

On Thu, Jan 28, 2021 at 01:18:55PM -0700, Mark Geib wrote:
> Adding socket:setopt(Socket, otp, debug, true) did not seem to have any effect. I captured a snippet from the log below which may help.
> 
> 2021-01-28T09:08:04.897919-08:00 [error] Supervisor: {local,ptc_6254_A_transport_sup}. Context: start_error. Reason: {badarg,[{prim_socket,nif_setopt,[#Ref<0.2135233317.2669019137.87142>,true,303,3002,#{interface => {172,24,4,30},multiaddr => {239,33,62,54},sourceaddr => {172,24,93,7}}],[]},{cse_socket,open_ssm_mc,
> 4,[{file,"/home/geib/git/cse.mpeg.invidi.pts_delta/apps/ipd/src/cse_socket.erl"},{line,87}]},{mpeg_ingest_rtp,init,1,[{file,"/home/geib/git/cse.mpeg.invidi.pts_delta/apps/ipd/src/mpeg_ingest_rtp.erl"},{line,49}]},{gen_server,init_it,2,[{file,"gen_server.erl"},{line,417}]},{gen_server,init_it,6,[{file,"gen_server.er
> l"},{line,385}]},{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,226}]}]}. Offender: id=ptc_6254_A,pid=undefined.

This is a bug that was accidentally fixed in a code cleanup on the master
branch, so it will not be present in OTP 24.0.

There are redundant checks on the map value, and one of them
is wrong; the size check is inverted.

This patch should fix the issue:

--- a/erts/emulator/nifs/common/prim_socket_nif.c
+++ b/erts/emulator/nifs/common/prim_socket_nif.c
@@ -10931,21 +10931,12 @@ ERL_NIF_TERM
esock_setopt_lvl_ip_update_source(ErlNifEnv*       env,
     struct ip_mreq_source mreq;
     char*                 xres;
     int                   res;
-    size_t                sz;
 #if defined(SOL_IP)
     int            level = SOL_IP;
 #else
     int            level = IPPROTO_IP;
 #endif
 
-    // It must be a map
-    if (!IS_MAP(env, eVal))
-        return enif_make_badarg(env);
-
-    // It must have atleast three attributes
-    if (!enif_get_map_size(env, eVal, &sz) || (sz >= 3))
-        return enif_make_badarg(env);
-
     if (!GET_MAP_VAL(env, eVal, atom_multiaddr, &eMultiAddr))
         return enif_make_badarg(env);

GET_MAP_VAL itself checks that it eVal a map, and the size is verified by the
fact that there are 3 of them.

And this would be a minimal patch; a 2=>1 chars patch:

--- a/erts/emulator/nifs/common/prim_socket_nif.c
+++ b/erts/emulator/nifs/common/prim_socket_nif.c
@@ -10943,7 +10943,7 @@ ERL_NIF_TERM
esock_setopt_lvl_ip_update_source(ErlNifEnv*       env,
         return enif_make_badarg(env);
 
     // It must have atleast three attributes
-    if (!enif_get_map_size(env, eVal, &sz) || (sz >= 3))
+    if (!enif_get_map_size(env, eVal, &sz) || (sz < 3))
         return enif_make_badarg(env);
 
     if (!GET_MAP_VAL(env, eVal, atom_multiaddr, &eMultiAddr))

/ Raimo


> 
> Mark.
> 
> 
> > On Jan 28, 2021, at 10:27 AM, Micael Karlberg <[email protected]> wrote:
> > 
> > Before the call to the setopt, add a call to socket:setopt(Socket, otp, debug, true).
> > 
> > /BMK
> > 
> > 
> > From: erlang-questions on behalf of Mark Geib
> > Sent: Thursday, January 28, 2021 6:24 PM
> > To: Erlang Questions
> > Subject: problem with new socket module setopt()
> > 
> > I am using the new socket module in order to join a source-specific multicast and receive datagrams. The following code fails with a “badarg” error on the socket:setopt() call. I have carefully checked the docs for R23, but can not find the problem.
> > 
> >     {ok, GroupAddr} = inet:getaddr(Group, inet),
> >     {ok, SourceAddr} = inet:getaddr(Source, inet),
> >     {ok, IfAddr} = inet:getaddr(Interface, inet),
> >     IpMreqSource = #{multiaddr => GroupAddr,
> >                      interface => IfAddr,
> >                      sourceaddr => SourceAddr},
> > 
> >     {ok, Socket} = socket:open(inet, dgram, udp),
> >     {ok, Port} = socket:bind(Socket, #{family => inet, port => Port, addr => GroupAddr}),
> >     ok = socket:setopt(Socket, ip, add_source_membership,  IpMreqSource),
> > 
> > I am using Erlang R23, latest release on debian-9 host.
> > Any suggestions would be appreciated greatly.
> > 
> > Mark.
> 



-- 

/ Raimo Niskanen, Erlang/OTP, Ericsson AB