Ticket #118 (closed enhancement: fixed)

Opened 11 years ago

Last modified 6 years ago

savannah: cannot specify port number in shell link

Reported by: palos Owned by: metux
Priority: major Milestone: 4.7
Component: mc-vfs Version: 4.6.1
Keywords: Cc:
Blocked By: Blocking:
Branch state: merged Votes for changeset: committed-master

Description (last modified by ossi) (diff)

Original: http://savannah.gnu.org/bugs/?18042

Submitted by:Palo Simo <palos>Submitted on:Tue 17 Oct 2006 12:51:17 PM UTC
Category:VFSSeverity:3 - Normal
Status:Ready For TestPrivacy:Public
Assigned to:Andrew V. Samoilov <sav>Open/Closed:Open
Release:current (CVS or snapshot)Operating System:All

Original submission:

this is a feature request:
include the ability to specify port number to connect to in the shell link to machine feature

Comment 1 by Andrew V. Samoilov <sav> at Fri 27 Oct 2006 12:15:02 PM UTC:

Hello, Palo.

Please test attached patch. This changes have to be documented in manuals before commit to CVS.

vfs/ChangeLog:

    * fish.c: Iterpret SUP.flags as port number if SUP.flags is not in 0, FISH_FLAG_COMPRESSED and FISH_FLAG_RSH. Weakness: port number

and r and C option cannot be used together.
Port 1 will be interpretted as 'C' option, port 2 as 'r'.

(fish_open_archive_int): Change for above.
(fish_fill_names): Likewise.

Comment 2 by Leonard den Ottolander <leonardjo> at Sat 28 Oct 2006 12:43:20 PM UTC:

> Weakness: port number
> and r and C option cannot be used together.
> Port 1 will be interpretted as 'C' option, port 2 as 'r'.


Do I understand correctly that a port number can be combined with C or r? And does one have to specify a port number if one uses 'r'? Please explain any syntax changes that you introduced.

From what I remember from earlier investigations to separate out the port from the other options we need to add an extra variable to some of the functions. Am I mistaken?

Maybe it's best first to decide on a new option syntax that allows all combinations and then implement that new syntax?

Comment 3 by Andrew V. Samoilov <sav> at Mon 30 Oct 2006 12:04:54 PM UTC:

> Do I understand correctly that a port number can be combined with C or r? And does one have to specify a port number if one uses 'r'? Please explain any syntax changes that you introduced.


No. Look at utilvfs.c:vfs_split_url().
No one syntax change. Only undocumented behaviour changed.
It is possible to use /#sh:user@host:6789 right now.
And fish uses port&1 as 'C' option and port&2 as 'r'.

Possible solution is to use 0x10000<<1 and 0x10000<<2 instead of 1 and 2.

> From what I remember from earlier investigations to separate out the port from the other options we need to add an extra variable to some of the functions. Am I mistaken?


You are right.

> Maybe it's best first to decide on a new option syntax that allows all combinations and then implement that new syntax?


I am not sure we can allow such redesign because of lack of manpower.

Comment 4 by Palo Simo <palos> at Mon 30 Oct 2006 08:11:45 PM UTC:

Thank you for the patch, it is working okay, great!
I do not use any of the other options, so I don't know if they are affected - I think you know whether or not ...

committed-master
changeset:a1b47185c952e9d31607d59420a0cb86e9f492ab

Attachments

11105-fish.c.port.patch (1.6 KB) - added by slavazanko 11 years ago.
0001-Port-number-in-shell-link-can-be-specified-now.patch (2.3 KB) - added by styx 11 years ago.
reworked for git am

Change History

Changed 11 years ago by slavazanko

Changed 11 years ago by styx

reworked for git am

comment:1 Changed 11 years ago by styx

  • Keywords review vote-styx added

see branch 118_port_number_in_shell_link

comment:2 Changed 11 years ago by winnie

  • Type changed from defect to enhancement
  • Milestone set to 4.7

This patch is an enhancement of fish and a very nice to have feature.

Maybe we should consider breaking once the rule and adding this enhancment already to a 4.6.X release...

comment:3 Changed 11 years ago by metux

  • Keywords vote-metux approved added; review removed
  • Status changed from new to accepted
  • Owner set to metux

added missing changelog entry and voting for the current branch.

comment:4 Changed 11 years ago by metux

  • Keywords committed-master added; approved removed
  • Status changed from accepted to testing
  • Resolution set to fixed
  • Description modified (diff)

comment:5 Changed 11 years ago by metux

  • Keywords vote-styx vote-metux removed

comment:6 Changed 11 years ago by styx

  • Status changed from testing to closed

comment:7 Changed 6 years ago by ossi

  • Description modified (diff)
  • Branch state set to no branch
  • Reporter changed from slavazanko to palos

comment:8 Changed 6 years ago by andrew_b

  • Keywords committed-master removed
  • Votes for changeset set to committed-master
  • Branch state changed from no branch to merged
Note: See TracTickets for help on using tickets.