On Tue, Jan 29, 2008 at 12:16:12PM +0530, Abhilash wrote:
> Sir,
>
> I Abhilash T.G and Arjun Asok of Amrita University Abhilash oso183 and
> Arjun oso211 have a fix for the bug 6440628 .
> we are attaching the diff.
Hmm. I am about to sound quite negative, but I aim to help; please
don't be disheartened.
> --- ipseckey.c.orig Tue Oct 2 19:45:09 2007
> +++ ipseckey.c Tue Jan 29 12:30:53 2008
> @@ -3053,7 +3053,11 @@
>
> if (argc == 0)
> return;
> - cmd = parsecmd(*argv++);
> + if (fopen(*(argv+1)){
> + cmd = parsecmd(*argv++);}
> + else{
> + ERROR(ep, ebuf, gettext("This is not a valid file name"));
> + }
Firstly, your style is rather strange. Please add whitespace where it
would help for readability. Also, the indentation makes it difficult to
see where blocks begin and end and you should try to mimic the style of
the file that you are modifying. Your change would be much more
readable as:
if (fopen(*(argv+1)) {
cmd = parsecmd(*argv++);
} else {
ERROR(ep, ebuf, gettext("This is not a valid file name"));
}
Secondly, your code doesn't compile. In the first case, a closing
parenthesis is missing from the "if (fopen..." line. In the second,
fopen() takes two arguments.
Thirdly, the ERROR() macro doesn't cease execution and the FATAL() macro
would be a better choice.
Fourthly, I don't like the message "This is not a valid file name".
The error message should be both exact and precise - it should include
the filename concerned and state exactly what the error was.
Finally, this check should take place during argument parsing in main().
Ceri
--
That must be wonderful! I don't understand it at all.
-- Moliere
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
URL:
<http://mail.opensolaris.org/pipermail/request-sponsor/attachments/20080129/edd50b6c/attachment.bin>
