Hi,

On Thu, 5 Jun 2008, Edward Z. Yang wrote:

> - I arbitrarily increased the allocation to arg. I think I could have
> gotten away with one less sizeof(*arg).

Yes.

> - There's a little bit of duplication with *arg++ = port; Dunno if we'd 
>   like to take it outside of the conditional.

Of course.  Repetition is not good.  I would prefer "(putty ? "-P" : 
"-p")".

> - I'm stupid, and a C n00b, so there are probably more problems with the 
>   patch.

Actually, it is pretty well written, even if it does not try to be as 
simple as possible.

For example, strstr() makes the strlen() comparison unnecessary.

> If things are good, it would be cool if someone am'ed it into the public 
> repository.

It would have been nicer to follow the hints in 
Documentation/SubmittingPatches, in particular inlining the patch, so that 
people can comment on it easily, and providing a sensible commit message 
so that applying it to "the public repository" (we have at least 3 of 
them, as far as msysGit is concerned) easy.

Ciao,
Dscho

Reply via email to