[patch #4492] Patch, allowing to set port to connect for "FIle transfer over SHell filesystem"
Leonard den Ottolander
savannah-bounces at gnu.org
Thu Oct 6 21:14:22 UTC 2005
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/
More information about the mc-devel
mailing list