Julian Gilbey <[EMAIL PROTECTED]> writes: > On Mon, Feb 13, 2006 at 10:29:40PM +0100, Marc 'HE' Brockschmidt wrote: >> Julian Gilbey <[EMAIL PROTECTED]> writes: >>> Patch attached. >> As this patch changes random other stuff all over the place, I'm not >> willing to apply it. Either you provide something less invasive or wait >> for me to do this myself this or next week. > Oops, sorry. The things which are not related or shouldn't be there: > - the "my $caller = (caller)[2];" line in get_ssh_connection; that's > cruft from my debugging > - the change of $& to $1 in line 1460: > $remote_ssh_port =~ s/^\s*(\d+)\s*$/-p $&/;
Yeah, plus a protocol addition which seems quite unnecessary to me.
> An altogether different approach would be to log the communication at
> the local end rather than at the remote end.
Actually, the only reason for having this code in the released version
is that I was too lazy to add/remove it when developing. I used it when
somehow the "server" didn't behave as expected, but as I don't expect
large changes to the codebase in the near future, I'm more inclined to
remove the whole log feature than loosing compatibility to the version
released in sarge.
>>> Other comments while I'm here:
>>> (1) You don't need the & in front of sub names; they'll work quite
>>> happily without it.
>> I know that quite well, but this is a matter of personal coding style.
> Indeed. The only significant difference is that if you want to use
> prototype checking, you cannot use the &.
I looked this up, BTW, and the reason for the & use is that it is used
for the "send" and "read" functions, which are also defined by
perl. Perl uses the global definition without the & in front of the
name.
>>> (2) -o for a remote ssh port number is a horrid choice: -o is normally
>>> an output file; -P would be much better (matching scp, for example).
>> -P was my first choice, but due to the limitations of the used
>> Getopt::Long module (which is not able to see the difference between -p
>> and -P) and the already existing "-p" short option for
>> --cache-passphrase, something else needed to be chosen. As all other
>> choices are equally bad, I used -o.
> Getopt::Long::Configure("no_ignore_case");
I remember trying this option, but something was weird [tm]. I'd need to
look this up, as I'm sure I ranted in IRC about it, but there were
definitely some problems.
Marc
--
BOFH #206:
Police are examining all internet packets in the search for a narco-net-traficer
pgpcUe2t9t8ko.pgp
Description: PGP signature
