Regular callback improvements

[JereLeppanen]

Regular callback is called only if there's no input from the client during the regular interval. Fix by calling the callback when the interval expires, regardless of input from the client.

[RobSanders]

Jere,
I'm finally making the time to review the pull requests against libcli and had a quick question for you - I'm not a Windows programmer at all, and I've seen some mixed answers if Windows supports the clock_gettime() functions. Have you tested this on Windows? Or just on *NIX platforms?

[JereLeppanen]

Only Linux. The man page says POSIX.1-2001, so it never really crossed my mind that it wouldn't be supported. I'll have to take another look at this.

[RobSanders]

I've been meaning to get access to a some sort of Windows platform just so I can verify that anything I do won't break it there. I've got multiple different NIX boxen to play with. Dave said awhile back that there were Windows users of libcli, but he himself was also NIX only.

[JereLeppanen]

I suppose one option would be to use Github's Windows runner. I've never tried that myself, so the only thing I know about it is that it exists.

So, there's no clock_gettime() on Windows, but at least with mingw, gettimeofday() could probably be used instead, even though it's not monotonic. Also, timeradd and timersub would be needed. These could maybe be copied from glibc, with the copyright statement. Or they could be implemented, although I'm not sure how to implement them without the appearance of having copied them from glibc or BSD.

Regardless, I wouldn't be able to actually test on Windows, because even without any of my changes, I'm not able to compile libcli with mingw or MSVC. Looks like it's been a while since libcli has compiled with mingw, and has probably never compiled with MSVC. Or I could be doing something wrong.

[RobSanders]

I think David said some time ago he wasn't sure who was using libcli on Windows anymore. Most of the users tend to be in the Linux/Unix world. I've poke briefly at seeing what is involved to get a Windows build system up, but just don't have the cycles to fully take that on. This is absolutely a good finding you have on the callback not firing unless there is no user interaction in that interval. I've been thinking about the use case you mentioned where a callback should be done more than once a second. Can you give a couple of examples? I'm wondering if we should perhaps not worry about that for now. Then we could simply use 'time()' (which we already are) instead of dealing with the cross-platform subsecond time commands. Another option would be to let a #define toggle between the new behavior you've added and leaving the old behavior active, then users can choose which way to go.

[JereLeppanen]

These are all viable options.

I was planning on implementing connection close triggered from another thread using the regular callback. With a one second timer, one has to wait for up to one second, which can be a long time. But this is for an internal testing and debugging utility, so I think we could just tolerate that.

So maybe we can just use time(). I'll rework these changes a bit.

[RobSanders]

I thought about threads, but my hesitation is that the core structure/design is not thread friendly - no locking, common state held in globals, and some absolute implicit assumptions on code flow. And for what libcli is tasked with doing, being thread aware isn't really needed.

[JereLeppanen]

I agree, there's no need to introduce concurrency in libcli. What is needed, though, is some way to stop cli_loop, and the regular callback can do that.

I updated my changes just now. So now it's one second resolution for regular callback, using time() (just like idle callback). Not very precise, but at least it's called eventually, regardless of client input.

[RobSanders]

This looks good. When testing it I must have done something 'odd'. Running under valgrind and I had tons of memory leaks - but haven't been able to replicate. So I suspect it was operator error on my part. Will be merging shortly. At this point I'm planning on a potential 1.10.8 release next week.

[RobSanders]

Accidentally merged to stable instead of the staging branch. I reverted the merge to stable, and will manually merge pr80 from my development machine into the staging branch and commit (w/changelog update)

[JereLeppanen]

Running under valgrind and I had tons of memory leaks

That does sound strange. This PR doesn't change any allocations. I added last_regular in struct cli_def, but I don't see how that could possibly cause any leaks. PR #79 added a realloc(), so that might be a suspect in case of any new memory leaks. But I'm not seeing any valgrind memcheck errors with either PR.

[RobSanders]

I think the leaks may have been due to my errors - I'd been bouncing back/forth between multiple branches and may have had clitest and libcli out of sync with each other. Was never able to recreate those errors outside of that one time. And I did multiple clean builds and runs to try and was not able to recreate the valgrind leaks. So I'm pretty sure it isn't the code.