Bug 25278 - [patch] bs accepts -s -c but not -sc
Summary: [patch] bs accepts -s -c but not -sc
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 4.2-STABLE
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2001-02-22 12:40 UTC by Andrew Stevenson
Modified: 2021-01-20 21:45 UTC (History)
0 users

See Also:


Attachments
file.diff (237 bytes, patch)
2001-02-22 12:40 UTC, Andrew Stevenson
no flags Details | Diff
file.diff (2.51 KB, patch)
2001-02-22 12:40 UTC, Andrew Stevenson
no flags Details | Diff
bs.1.usage.patch (930 bytes, patch)
2002-06-09 17:22 UTC, Andrew Stevenson
no flags Details | Diff
bs.2.getopt.patch (1.49 KB, patch)
2002-06-09 17:28 UTC, Andrew Stevenson
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Stevenson 2001-02-22 12:40:01 UTC
The bs program accepts -s and -c as arguments but not -sc. bs was using its own
argument passing routines so I changed it over to getopt. While I was at it I
added a usage function, changed the program name in the usage message from
battle to bs (as that is what it is installed as nowdays), changed the exit
value from 1 to EX_USAGE when exiting because of invalid arguments, sorted the
included header files as per style(9) and added -Wall to the makefile.

I've tried to keep the same programming style as the existing code.

How-To-Repeat: 
bs -sc (it runs but the c is ignored)
Comment 1 dd freebsd_committer 2001-06-19 02:11:46 UTC
State Changed
From-To: open->feedback

The patch looks good, except it's difficult to review because you 
chose to do #include reorganization, style fixes, or play function 
games (I can't tell which) along side functional changes.  Please 
submit this as two different patches.
Comment 2 dd freebsd_committer 2001-06-19 02:11:46 UTC
Responsible Changed
From-To: freebsd-bugs->dd

I'll take care of this provided that the originator can split this up 
into managable chunks.
Comment 3 Andrew Stevenson 2001-06-19 11:50:17 UTC
On Mon, 18 Jun 2001 dd@FreeBSD.org wrote:

> The patch looks good, except it's difficult to review because you
> chose to do #include reorganization, style fixes, or play function
> games (I can't tell which) along side functional changes.  Please
> submit this as two different patches.

OK here's a patch to just add a usage function and convert bs to using
getopt.

Thanks,

Andrew
Comment 4 Andrew Stevenson 2001-06-19 11:59:50 UTC
Here's a patch to sort the headers. It assumes the previous patch has been
applied.

Thanks,

Andrew

--- bs.usage/bs.c	Tue Jun 19 20:38:38 2001
+++ bs.headers/bs.c	Tue Jun 19 20:54:12 2001
@@ -9,15 +9,15 @@
  * $FreeBSD: src/games/bs/bs.c,v 1.9 2000/02/21 03:07:31 billf Exp $
  */

+#include <assert.h>
+#include <ctype.h>
 #include <ncurses.h>
 #include <signal.h>
-#include <ctype.h>
-#include <assert.h>
 #include <stdlib.h>
-#include <unistd.h>
-#include <time.h>
 #include <string.h>
 #include <sysexits.h>
+#include <time.h>
+#include <unistd.h>

 #ifndef A_UNDERLINE	/* BSD curses */
 #define	beep()	write(1,"\007",1);
Comment 5 Andrew Stevenson 2001-06-19 12:02:55 UTC
OK, here is a patch to add -Wall to the Makefile.

--- Makefile.orig	Tue Jun 19 21:00:39 2001
+++ Makefile	Tue Jun 19 20:52:18 2001
@@ -2,6 +2,7 @@

 PROG=   bs
 MAN=   bs.6
+CFLAGS+=-Wall
 DPADD=  ${LIBNCURSES} ${LIBMYTINFO}
 LDADD=  -lncurses -lmytinfo
 HIDEGAME=hidegame
Comment 6 dima 2001-06-20 01:17:39 UTC
<andrew@ugh.net.au> writes:
> 
> 
> On Mon, 18 Jun 2001 dd@FreeBSD.org wrote:
> 
> > The patch looks good, except it's difficult to review because you
> > chose to do #include reorganization, style fixes, or play function
> > games (I can't tell which) along side functional changes.  Please
> > submit this as two different patches.
> 
> OK here's a patch to just add a usage function and convert bs to using
> getopt.

Did you forget to attach something? :-)

> 
> Thanks,
> 
> Andrew
>
Comment 7 dima 2001-06-20 01:18:39 UTC
<andrew@ugh.net.au> writes:
> OK, here is a patch to add -Wall to the Makefile.
> 
> --- Makefile.orig	Tue Jun 19 21:00:39 2001
> +++ Makefile	Tue Jun 19 20:52:18 2001
> @@ -2,6 +2,7 @@
> 
>  PROG=   bs
>  MAN=   bs.6
> +CFLAGS+=-Wall

I don't know if you're using -currnet, but the new WARNS= stuff is
preferred to specifying warning flags in CFLAGS.

>  DPADD=  ${LIBNCURSES} ${LIBMYTINFO}
>  LDADD=  -lncurses -lmytinfo
>  HIDEGAME=hidegame
> 
>
Comment 8 Andrew Stevenson 2001-06-20 01:24:00 UTC
On Tue, 19 Jun 2001, Dima Dorfman wrote:

> > OK here's a patch to just add a usage function and convert bs to using
> > getopt.
>
> Did you forget to attach something? :-)

Ooops. Lets try again...

--- bs/bs.c	Tue Jun 19 20:09:29 2001
+++ bs.usage/bs.c	Tue Jun 19 20:38:38 2001
@@ -17,6 +17,7 @@
 #include <unistd.h>
 #include <time.h>
 #include <string.h>
+#include <sysexits.h>

 #ifndef A_UNDERLINE	/* BSD curses */
 #define	beep()	write(1,"\007",1);
@@ -1110,59 +1111,53 @@
     return(sgetc("YN") == 'Y');
 }

-static void do_options(c,op)
-int c;
-char *op[];
+static void usage()
 {
-    int i;

-    if (c > 1)
-    {
-	for (i=1; i<c; i++)
-	{
-	    switch(op[i][0])
-	    {
-	    default:
-	    case '?':
-		(void) fprintf(stderr, "Usage: battle [-s | -b] [-c]\n");
-		(void) fprintf(stderr, "\tWhere the options are:\n");
-		(void) fprintf(stderr, "\t-s : play a salvo game\n");
-		(void) fprintf(stderr, "\t-b : play a blitz game\n");
-		(void) fprintf(stderr, "\t-c : ships may be adjacent\n");
-		exit(1);
-		break;
-	    case '-':
-		switch(op[i][1])
-		{
+	(void) fprintf(stderr, "Usage: bs [-s | -b] [-c]\n"
+						  "\tWhere the options are:\n"
+						  "\t-s : play a salvo game\n"
+						  "\t-b : play a blitz game\n"
+						  "\t-c : ships may be adjacent\n");
+	exit(EX_USAGE);
+}
+
+static void do_options(argc,argv)
+int argc;
+char *argv[];
+{
+    int c;
+
+	while ((c = getopt(argc, argv, "bcs")) != -1) {
+		switch (c) {
 		case 'b':
-		    blitz = 1;
-		    if (salvo == 1)
-		    {
-			(void) fprintf(stderr,
-				"Bad Arg: -b and -s are mutually exclusive\n");
-			exit(1);
-		    }
-		    break;
-		case 's':
-		    salvo = 1;
-		    if (blitz == 1)
-		    {
-			(void) fprintf(stderr,
-				"Bad Arg: -s and -b are mutually exclusive\n");
-			exit(1);
-		    }
-		    break;
+			if (salvo == 1)
+			{
+				(void) fprintf(stderr,
+						"Bad arg: -b and -s are mutually exclusive\n");
+				exit(EX_USAGE);
+			} else {
+				blitz = 1;
+			}
+			break;
 		case 'c':
-		    closepack = 1;
-		    break;
+			closepack = 1;
+			break;
+		case 's':
+			if (blitz == 1)
+			{
+				(void) fprintf(stderr,
+						"Bad arg: -s and -b are mutually exclusive\n");
+				exit(EX_USAGE);
+			} else {
+				salvo = 1;
+			}
+			break;
+		case '?':
 		default:
-		    (void) fprintf(stderr,
-			    "Bad arg: type \"%s ?\" for usage message\n", op[0]);
-		    exit(1);
+			usage();
 		}
-	    }
 	}
-    }
 }

 static int scount(who)


Thanks,

Andrew
Comment 9 Andrew Stevenson 2001-06-20 01:25:43 UTC
On Tue, 19 Jun 2001, Dima Dorfman wrote:

> I don't know if you're using -currnet, but the new WARNS= stuff is
> preferred to specifying warning flags in CFLAGS.

No, only stable. If someone running current wants to change this I'm quite
happy. Thanks for pointing it out,

Andrew
Comment 10 Andrew Stevenson 2002-06-09 17:22:07 UTC
Well I've rewritten the changes to try and minise the diffs...not too
successfuly however but hopefully enough to see them committed.

Here is a patch to split usage off into a seperate function.

(other patches to follow)

Thanks,

Andrew
Comment 11 Andrew Stevenson 2002-06-09 17:28:03 UTC

On Mon, 10 Jun 2002, Andrew wrote:

> (other patches to follow)

Ok here is a patch to get bs to use getopt. The diff looks messy but it
basically changes the guts of do_options from parsing command line options
itself to calling getopt. If you apply the patch then compare the
functions manually you can see what it happening.

This patch actually fixes the problem I was having.

Thanks,

Andrew
Comment 12 dd freebsd_committer 2005-06-14 11:19:35 UTC
Responsible Changed
From-To: dd->freebsd-bugs

Sorry, I don't have time to do this right now. If you (the submitter) 
are still interested, perhaps sending an updated patch to a mailing 
list would stir some activity. 

My apologies for dropping the ball on this for so long.
Comment 13 Mark Linimon freebsd_committer freebsd_triage 2008-03-01 19:43:14 UTC
State Changed
From-To: feedback->suspended

It does not look like anyone is working on this right now.
Comment 14 Antoine Brodin freebsd_committer 2008-05-21 19:08:55 UTC
State Changed
From-To: suspended->closed

Change state from suspended to closed: games/bs has been 
removed from FreeBSD 5 years ago.