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

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
- 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

> +	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

Please supply a proper patch.


Reply to this item at:


  Message sent via/by Savannah

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]