Scotty Improvements #1&2 - modernize build system, fix configure for -address and -port

[jorge-leon]

I am starting to work on the Scotty build system to update it to current TEA (3.10), taking into account the comments in #7 .

[jorge-leon]

At this moment I have two branches in my clone of FlightAwares Scotty repository which build on Debian Jessie, FreeBSD 11 and Mac OS X Yosemite with Tcl stubs enabled.

• tnm-only-build: builds the Tnm package and (fails to build) scotty
• tki-only-build: builds the Tkined package and tkined

Some of the included tests for Tnm run, sunrpc beeing notably an exeption since it segfaults.
The results of the tests differ on different platforms and look strange on Mac OS X.

tkined starts up without the toolbar, so is not really usable. The problem might be with--
hardcoded paths in the libraries, which do not match TEA's idea about standard locations.
This was just a missing tkined.default file in the install target.

Interestingly, tkined segfaulted on a FreeBSD w/o X-org installed in the call to TkpGetSubfonts(),
when launched via ssh and X11 forwarding.

---

I was able to contact J. Schönwalder and Mark Newnham. Ideas is to merge the work of Mark
(icons, mostly) and to link history from 2000 to now (Scotty page in Tcler's Wiki).

[jorge-leon]

At this moment I have a fork of the FlightAware scotty repository which hopefully complies with the
bounty requirements for both Scotty bounties.

Scotty has been split into two individual directories: tnm and tkined. Inside each directory you can do:

$ autoconf
$ ./configure options ...
$ make
$ make install

Tnm has received most love, you can run make test to run the testsuite, which should terminate successfully on Debian jessie, FreeBSD 11 and Mac OSX 10.10.5.

The TODO file in each directory lists all problems encountered with the software, there will be much
more though...

[jorge-leon]

On Tnm::udp:

udp# configure does not set -address or -port on purpose. udp# send has two different use cases:

1. You want to send packets to different host/port combinations with your specific udp# object.
2. You want do send packets always to the same host/port combination.

In case 2. it is supposed to use udp# connect host port first. The udp# object is then said to
be 'connected'. All future invocations of udp# send must not specify hostand port and always
go to the same destination.

Note that the remote data hostand port are not stored in the udp# object, but rather in the
socket associated with the udp# socket.

The very simple approach of my patch is to:

• store -address
• store -port
• if both are (re)set do a udp# connect internally with the configure'd data.

Notes:

• The current approach of scotty does not allow to re-configure a connected udp# object. You
  must destroy it and create a new one.

• Order of configure items is important. You must first configure -myaddress and -myport and
  then -address and -port.

[jorge-leon]

I just created a pull reqest with what I think is a completion of the given tasks.

Scotty needs some love though, but with the new build this should be easier to manage now.

For the upd bounty I have ideas for alternative implementations/funcionality, but maybe you try this
on out first.

[resuna]

It needs some cleanup. I was going through the commits and noticed that there were some places where files were moved to backup files, and then new copies of files inserted. There's three backup files actually checked in to the repo:

tkined/generic/tkiPort.h~
tnm/generic/tnmPort.h~
tnm/tests/dns.test~

Can you clean that up, or at least verify that noting of value was lost?

[jorge-leon]

Oh! Fixed that one, nothing lost here.

Will countercheck right now, if all files are here against the original version.

[resuna]

"make test" in Tnm should be pulling in the version in the source tree, not the installed version, because that's what you're testing.

Tests began at Mon Feb 27 20:05:43 UTC 2017
dns.test
couldn't read file "/usr/local/lib/Tnm3.0.2/library/init.tcl": no such file or directory
icmp.test
command "Tnm::dns" already exists
job.test
command "Tnm::dns" already exists
map.test
command "Tnm::dns" already exists

[jorge-leon]

Files are all there.

"make test":

The original scotty works that way, it explicitely requires you to install first. The reason is, you need
nmicmpd and nmtrapd install setuid root.

However I think you are right and will rewrite this part of the Makefile accordingly. Do you think
it is okay to set these programs setuid root in the source tree first and then install them into their
final destination?

Note: nmtrapd seems not to be used by the tests.

[resuna]

Ah, OK.

There's a "make check" that seems to run out of the current directory, but it is SEGV-ing in scotty for me.

$ make check
*** Signal 11

Stop.

[jorge-leon]

That is, because it uses the C - scotty. C - scotty currently segfaults, I have added tclscotty to
compensate for now. See tnm/TODO.

[resuna]

Seems like there's a bit of work to do on this still.

[jorge-leon]

Yes, most of the items in the TODO files are already present in the original source from where I started,
e.g. scotty spins about a minute on my machine before presenting a prompt, ... So I focussed on getting the library to compile and all tests running (with tclscotty).

I need feedback where the priorities are. Do you need smx? libsmi integration?

[resuna]

We need something that is functionally complete and has no regressions on FreeBSD and Linux compared with the FlightAware version. What items in TODO are in the code you started from, and what items are new?

Mac OS X may be required too.

[jorge-leon]

To find out exactly what are the differences I will run the tests and compare. May I suppose you are
using FreeBSD 11 as primary development and test environment? Please specify which Linux
distribution in which version you are using, and with which kernel version.

Is it ok to start testing with tclscotty? The original C scotty has no stub support and is quite a rewrite of
of tclsh. So I don't know how long it takes to get this stable. The only real difference of C scotty
with respect to package require Tnm is a modified event loop which prioritizes file events.

[resuna]

If you have to run the tests using tclscotty, then the Makefile should probably be running them that way too. Possibly the original scotty could be deprecated, if it's that obsolete. I'll check whether that's actually required.

[jorge-leon]

Please comment on development and testbed environments.

[jorge-leon]

Creating a new C-scotty was easier then I thought - it's already commited to my scotty repository.

The Tnm library hardcodes the absolute path to itself and uses it to construct further paths from
where e.g. library/init.tcl is loaded. This might be messy, but it is in the original scotty code and
I think we should check functionalty and regression against the current code base before
(potential incompatible) improvements are made.

Find attached test logs made with a current checkout of FlightAwares' scotty repository on a FreeBSD 11
VirtualBox. test.log is created with make check and is incomplete because the sunrpc tests
segfault. So I sourced each test file manually into the scotty binary and logged the results into
test_individually.log. Please compare this with your results to see, if we can use this as a starting
point.

After this I installed my current version and run tests with make test, logging into
test_Tnm3.0.2.log. This would be a test of the Tnm library alone.

make check uses the scotty binary created in the source tree, its output can be found in
test_scotty3.0.2.log.
test.log.txt
test_individually.log.txt
test_scotty3.0.2.log.txt
test_Tnm3.0.2.log.txt

[resuna]

FreeBSD 10.3, 11.0
macOS Sierra
Linux - investigating

[resuna]

Ubuntu 16.10

[jorge-leon]

Can we agree on starting from FreeBSD 11. I would like you to run make check with the original
FlightAware version in your environment and post the result, so we can compare.

[resuna]

Most of our environment is 10.3. I'm not sure I have an 11 environment I can test in.

[jorge-leon]

No problem: I'll set it up on 10.3.

[resuna]

make-check-original-freebsd-10.3.txt

[jorge-leon]

Do you have a 32 bit or 64 bit envorinment? The mib-7.14 mib format test (failure) would probably
indicate the latter. Please post uname -a (mangled at your will) so I can set up accordingly.

[resuna]

VM I'm using for testing is:

FreeBSD vm-resuna1 10.3-RELEASE FreeBSD 10.3-RELEASE #0 r297264: Fri Mar 25 02:10:02 UTC 2016 root@releng1.nyi.freebsd.org:/usr/obj/usr/src/sys/GENERIC amd64

I noticed the mib format failures and they seem not a big deal. Same with most of the netdb failures. I don't think the bounty needs that kind of existing stuff to be cleaned up.

[resuna]

The new code doesn't seem to be pulling in configuration right, in particular it's redefining socklen_t when it shouldn't be.

unix/config.h is being generated with #define HAVE_SOCKLEN_T 1

But it looks like nothing is pulling that info into tnmInt.h so it's going:

#ifndef HAVE_SOCKLEN_T
typedef size_t socklen_t;
#endif

Which is why I think... when you build you get a bunch of stuff like this:

In file included from unix/nmtrapd.c:36:0:
/usr/include/sys/socket.h:609:9: note: expected 'socklen_t *' but argument is of type 'int *'
ssize_t recvfrom(int, void *, size_t, int, struct sockaddr * __restrict, socklen_t * __restrict);

[jorge-leon]

Great :)
I have been cleaning up things I encounted on the way wherever it was obvious how to do it.
And there is a lot more to do to get scotty state of the art of todays Internet and operating systems.
e.g. interface detection and enumeration, which is related to IP address detection. scotty
relies on /etc/hosts /etc/networks, etc. files which are not maintained coherently on todays systems.

[resuna]

Aha, just checked the old version, it had the same problem. I'll work with Karl on that.

[jorge-leon]

Original code uses config.h, however it did not pick it up everywhere correctly so I disabled config.h
generation and all #defines go on the compilers commandline. Personally I thought investigating in
depth at some moment and switch back to config.h.

Anywhere: HAVE_SOCKLEN_T is definitely defined on the compiler commandline. I will go through
the source and check things.

Note that virtually no code has been changed. Just the autconf/configure/Makefile.in stuff.

[jorge-leon]

I understand, that OOM is not the case here. Your patch resolves the situation of a missing domain part in the return value of Tcl_GetHostName().

In our test environments it is unlikely to get an OOM. But how should situations be handled, where memory is allocated but the result is not checked?

[resuna]

We can fix that possible failure mode, yes. I wouldn't be surprised if there were not lots of other unchecked allocations in the existing legacy code. The underlying problem here is the regression in Linux (and possibly Windows) caused by checkin e17ceb3. That needs to be reverted or have some kind of fallback that gets the domain.

[mutability]

ckstrdup calls ckalloc; ckalloc panics if it is OOM; caller doesn't need to do anything?

[resuna]

The behavior of "info hostname" on Linux and FreeBSD is also inconsistent.

[jorge-leon]

I propose to initialize to recognizable invalid values, just like done with the user name or the os.
A hostname of "unknown" and an empty string for the domain should do.

The original code uses the rather not standard res_ninit() function, which essentially reads /etc/resolv.confor the environment variable LOCALDOMAIN. https://linux.die.net/man/3/libbind-resolver

Tcl_GetHostName() on unix uses either uname(2) or gethostname(). The result with respect to the domain part depends on configuration, libc and operating system.

So: "unkown" and ""?

[resuna]

Hostname is not an issue. Whatever is returned by Tcl_GetHostName() is absolutely fine. It seems to be all [info hostname] does.

It doesn't need to work on Windows, at all, but Linux, I think if we can have it work there we should have it work there. Possibly something like:

if(p)
    Tcl_SetVar2(interp, "tnm", "domain", ++p, TCL_GLOBAL_ONLY);
#ifdef linux
else
    Tcl_SetVar2(interp, "tnm", "domain", linux_res_ninit_hack(), TCL_GLOBAL_ONLY);
#endif

[jorge-leon]

It is absolutely no problem to revert to the original version. In fact I stumbled about res_ninit() only
because musl libc does not provide it. If we revert, scotty will not compile on Alpine Linux, that's it.

But I can't stop ranting:

If your system is not configured respectively (resolv.conf has no domain entry), res_ninit() will not give you a domain either.

IMHO it should be documented, how to set up the system on each OS to get the right thing out of Tcl_GetHostName() and rely on one centralized functionality which can be managed in the core instead of using just another heuristic which depends on os specific features.

[jorge-leon]

On Windows Tcl_GetHostName() uses the GetComputername() function, which returns the NetBIOS name of the local computer. So it is not supposed, that there be a domain name attached.

I think I stand corrected: Let's get the hostname with Tcl_GetHostName and the Internet domain name with a function provided by the Internet savvy Tnm library.

[resuna]

The machine I'm testing this on has a "search" line in resolv.conf, but no "domain" line. This is what dhcpd puts there on both FreeBSD and Linux.

Looking at the source to the version of res_ninit() on OS X and in glibc, in both cases it seems to be using the "search" line. In fact it uses search by preference, only falling back to domain if search isn't there.

So there's no configuration issue. These systems are correctly configured.

[resuna]

If you want to write the code to pull in the domain name using Tcl code from the Tnm library, as a fallback if Tcl_GetHostName() doesn't return a domain part, that's probably OK. In that case you should probably leave tnm(domain) unset in the "C" code and check on it later once you're mucking about in Tcl anyway.

[resuna]

Wouldn't the proper fix be to have an autoconf check for res_ninit() and make the code you removed from the Linux platform-dependent routine conditional on HAVE_RES_NINIT?

[jorge-leon]

That would be a possibility. Also there are workarounds to find out the default domain if res_ninit() is not available.

Sidenote: Read what resolv.conf(5) has to say about the search line...

[jorge-leon]

As of now I have reverted the code for setting the tnm(domain) variable completely to the original, to avoid more interference when testing.

I have done a general cleanup to eliminate most compiler warnings and put config.h into place. The build is now more quiet.

The netdb.tests suite gets killed by FreeBSD because it eats up all memory when running the netdb ip tests. Please disable netdb-6.6 and netdb-6.7 e.g. by putting in a "knownBug" constraint, or using my version. This allows the whole testsuite to finish.

I attach my respective logs from FreeBSD 10.3/amd64 and Ubuntu 16.10/i686

make_check_freebsd10.3.log.txt
make_check_ubuntu16.10.log.txt

[resuna]

Our Linux boxes are x86_64/amd64

[resuna]

I'll get with Karl this afternoon to see about testing your fork.

I notice a bunch of errors on the FreeBSD side related to UDP sockets with an address and port. Are they related to the work you're doing with the UDP bounty?

Eg:

==== udp-11.2.3 udp configure/send: send to configured, connected FAILED
==== Contents of test case:

    catch {rename udp# {}}
    set r [udp create -myaddress 127.0.0.1 -myport $::SOME_PORT]
    rename [udp create -myaddress 127.0.0.1 -myport $::OTHER_PORT] udp#
    udp# connect 127.0.0.1 $::OTHER_PORT
    udp# configure -address 127.0.0.1 -port $::SOME_PORT
    udp# send "nase"
    $r receive

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: can not bind socket: address already in use
    while executing
"udp create -myaddress 127.0.0.1 -myport $::SOME_PORT"
    ("uplevel" body line 3)
    invoked from within
"uplevel 1 $script"
---- errorCode: POSIX EADDRINUSE {address already in use}
==== udp-11.2.3 FAILED

[jorge-leon]

Good to hear. If you are ready, please consider to look at the udp sendimprovements also.

The udp errors in the test are unrelated to the changes. The tests are made for a Linux network stack. FreeBSD apparently does not allow to send and receive on the same udp port at the same time.

There are other test with similar problems, e.g. using an arbitrary IP number out of 127.0.0.0/8, which works on Linux but not on other operating systems, unless you define an IP alias on the loopback interface.

[resuna]

OK, I've reviewed the UDP errors in the tests. I noticed this comment:

Note: these use the same udp endpoint to send and receive. Might just rewrite the tests.

[jorge-leon]

Comment is mine. Tests up to udp-9.1 are from the original scotty. I didn't want to modify them much in order to maintain comparability.

Testgroups 10 and 11 are for the new functionality. The tests work on the Linux network stack, I will adapt them to work on FreeBSD too.

[resuna]

If it's not a huge push-up, I think it would be better to make the previous tests consistent rather than not testing them on FreeBSD, since that's our primary platform and you're modifying 10 and 11 anyway.

[jorge-leon]

Updated udp tests and ran them on FreeBSD10.3/amd64, Debian Jessie, Ubuntu 16.10, MacOSX Yosemite.

Will need to update the documentation for the new udp# send functionality, since not all operating systems allow to sendto() from an already connected() socket.

With my recent cleanups tkined builds fail on MacOSX. Currently looking into it. Fixed by adding a missing HAVE_CONFIG_H stanza.

[resuna]

Coincidentally, I just made some changes in the base scotty to make it handle trailing . in host name lookups like the stock resolver. Can you pull these patches in?

flightaware/scotty@85022ae
flightaware/scotty@a10621d
flightaware/scotty@d8703ae

PS: It's using res_mkquery/res_send instead of res_search to provide more control over the process, per Karl.

PPS: Thanks for the Mac fix, I'll check it out today.

[resuna]

On Sierra I get these errors.

Undefined symbols for architecture x86_64:
  "_Tcl_GetErrorLine", referenced from:
      _GeneratorCmd in tnmSnmpTcl.o
      _WalkTree in tnmMibTcl.o
ld: symbol(s) not found for architecture x86_64

Edit: See http://wiki.tcl.tk/22108

[resuna]

After applying the 22108 fix it builds and seems to work, tkined has a bit of a glitch in the menus.

If you can't see an easy fix, don't worry about it.

[jorge-leon]

I just have pulled in your commits from #26 (comment), do you have one for _Tcl_GetErrorLine handy?

Maybe this will get me to go with Tcl8.5 too.

In a minute I will test my recent patches for Tnm::dns on MacOSX and take a look on the menu issue.

[resuna]

I stuck the mod from the wiki into the files with the problem, but I haven’t sat down and gone “well, it should probably be in tnmSnmp.h or maybe config.h or blah blah blah”... you know, what’s the *right* place to put the thing. Since you’ve had your nose in this more than me lately, I figure I’ll defer that to your judgement.

[jorge-leon]

Thats fine with me.

[jorge-leon]

With the last commit, I could build and run tests of scotty on:

• Debian Wheezy (7.11)
• Debian Jessie (8.7) (32bit)
• FreeBSD 10.3
• FreeBSD 11.0 (32bit)
• Ubuntu 16.10

The missing operating systems from your list would essentially be: Debian Sid, macOS Sierra.
You asked for Debian 7.9 and Jessie 8.2 too: please confirm if you want me to test them. These are
non updated version of Wheezy and Jessie.

Other OSses tested:

• macOS Yosemite, 10.10.5
• Slackware LInux 14.2
• NetBSD 7.0.2 (Tnm::dns will not work here w/o touching the code)
• Oracle Solaris 11.3 (build needs help)

I'm in contact with Minix3, and hope they get me a working rpcgen/rcp implementation to test here too.

---

The upd send command fix and new functionality is in place and gets tested on all systems. The test file udp.test has a lists of systems, where udp connect and udp send with configured destination must not be mixed.

[resuna]

I've merged this into flightaware/scotty master.

[resuna]

Do we have stub support in this release?

[jorge-leon]

Everything is compiled with USE_TCL_STUBS and linked against tclstub8.6.