[patch #4492] Patch, allowing to set port to connect for "FIle transfer over SHell filesystem"

[Leonard den Ottolander <savannah-bounces gnu org>]

Follow-up Comment #1, patch #4492 (project mc):

A couple of general comments:

- Please use the -p switch for diffs. This way it's obvious which function is
affected.
- Do not remove empty lines (at least not as integral part of a functional
patch). They increase readability.
- Do not use C99 style comments ("//").

And with regard to the code:

> +	char port[255];
This is a rather big string to store the representation of an integer. Give
it a sensible size or allocate it dynamically (and don't forget to release
it).

> +    if (flags > (FISH_FLAG_RSH|FISH_FLAG_COMPRESSED))
> +	SUP.port = flags;
> +    else
> +	SUP.port = 22;
>      SUP.flags = flags;

This is an ugly hack. Firstly the flags have nothing to do with the port
number, secondly it ignores the case where port < 4 or more flags are defined
and thirdly you assign a bogus value to SUP.flags if you do use flags to
represent the port number.

> +	if (SUP.flags > (FISH_FLAG_RSH | FISH_FLAG_COMPRESSED))//port

And here you make the same ugly assumption. Why don't you use SUP.ports
instead?

Please supply a proper patch.


    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?func=detailitem&item_id=4492>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/