Skip to content

[YDBTest#598] Fix ZSOCKET(sock,"OPTION",index) implementation, add missing options

Erno Zalka requested to merge ern0_ydb_fork/YDB:fix_zsocket_report_options into master

Background

Release note says:

The $ZSOCKET() function supports an "OPTIONS" keyword which takes an index argument and returns a string of the OPTIONS previously specified for the selected socket. The string may not exactly match the string originally specified but has the same meanings.

The bug revealed during creating the tests, this quote is from the thread about it:

I think I was able to reproduce your issue with the following simple test case. Below is using GT.M V7.0-003.

The simple test case goes (copied from the thread):

GTM>set s="socket"  open s:(CONNECT="tynt:22:TCP")::"SOCKET"

GTM>use s:(options="KEEPALIVE=101,KEEPCNT=102,KEEPIDLE=108")

GTM>use $p

GTM>write $zsocket(s,"keepalive",0),!
101;1

GTM>write $zsocket(s,"keepcnt",0),!
102

GTM>write $zsocket(s,"keepidle",0),!
108

GTM>write $zsocket(s,"options",0),!
KEEPALIVE=108

The proper result of write $zsocket(s,"options",0) would be KEEPALIVE=101,KEEPCNT=102,KEEPIDLE=108, echoing the values set by the USE command. Instead, it mixes first option name with last value.

Also discovered that use s:(options="KEEPCNT=0") and use s:(options="KEEPCNT=128") causes assert error.

Issue

The implementation of the $ZSOCKET(s,"options",0) has more problems:

  • it's incomlpete, only reports KEEPALIVE, KEEPCNT and SNDBUF,
  • it produces improper result when more options are set.

There's an assert(FALSE) in sr_port/iosocket_tcp_keepalive.c, which is unnecessary.

Fix

Using some elements from old code, the implementation of OPTIONS have been rewritten:

  • replaced complete block of case zsocket_options
  • added a 2-round loop,
  • in the first round, the required string space gets calculated (variable: determined_length),
  • between the rounds, the free space is checked,
  • in the second round, the result string is assembled,
  • after the second round, dst->str.addr and dst->str.len is set, as original code did.

Inside the loop, there is a zsocket_options_item() call for each option: KEEPALIVE, KEEPIDLE, KEEPCNT, KEEPINTVL, SNDBUF (aka. socketptr->iobfsize). This function calculates the length of the result in round 1, and collects result values in round 2, for the keyword and its structs received as arguments.

Removed unnecessary assert(). Details in this thread

Edited by Erno Zalka

Merge request reports