Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 25 Jul 1996 16:18:53 +0200
From:      Zahemszky Gabor <zgabor@code.hu>
To:        FreeBSD-gnats-submit@freebsd.org
Subject:   bin/1429: sh(1) and getopts
Message-ID:  <199607251418.QAA01059@zg.CoDe.hu>
Resent-Message-ID: <199607251400.HAA00141@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help

>Number:         1429
>Category:       bin
>Synopsis:       sh(1) and getopts
>Confidential:   yes
>Severity:       serious
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Thu Jul 25 07:00:02 PDT 1996
>Last-Modified:
>Originator:     Zahemszky Gabor
>Organization:
>Release:        FreeBSD 2.1.0-RELEASE i386
>Environment:

	2.1R CD from Walnut Creek

>Description:

	The builtin getopts command of sh(1) is not the POSIX getopts as the
	manual says.
	1) The OPTIND variable should be 1 at the initialization
	2) The OPTIND variable should be handle in the getopts cycle, not
	only at the end
	3) On an illegal option, the OPTARG variable should set to the
	wrong option
	4) On an option, which has missing the argument, the OPTARG should
	set to the option, and the ``getopts options var'' list's
	var has to set to : instead of ?
	5) If the options begins with :, error messages shouldn't print.
	6) We should run more and more getopts in one shell invocation, with
	setting OPTIND to 1
	7) getopts should handle other options, not only the "$@", so
	we can use it: getopts options var optionlist

>How-To-Repeat:

	

>Fix:
	
	Here is my patches, which made getopts do 1-6.  (The 7 is not too
	hard, I think, but I havent enough time to look into the code
	Only one another note:
	To make the :options stuff working, this patch uses a simple local
	variable and a test on every invocation of getopts.  If it would be
	better (it's a bit more quick), we should use a static local
	variable, and the test made in only getopts initialization.

	Here are the two needed diffs, and after it the diff with the
	static variable:
===========================================
*** var.c.orig	Tue May 30 02:07:24 1995
--- var.c	Thu Jul 25 15:39:37 1996
***************
*** 76,81 ****
--- 76,82 ----
  struct var vifs;
  struct var vmail;
  struct var vmpath;
+ struct var voptind;
  struct var vpath;
  struct var vps1;
  struct var vps2;
***************
*** 92,97 ****
--- 93,99 ----
  	{&vifs,	VSTRFIXED|VTEXTFIXED,		"IFS= \t\n"},
  	{&vmail,	VSTRFIXED|VTEXTFIXED|VUNSET,	"MAIL="},
  	{&vmpath,	VSTRFIXED|VTEXTFIXED|VUNSET,	"MAILPATH="},
+ 	{&voptind,	VSTRFIXED|VTEXTFIXED,		"OPTIND=1"},
  	{&vpath,	VSTRFIXED|VTEXTFIXED,		"PATH=:/bin:/usr/bin"},
  	/*
  	 * vps1 depends on uid

------
===========================================
*** options.c.orig	Tue Oct 10 02:04:38 1995
--- options.c	Thu Jul 25 16:00:22 1996
***************
*** 346,354 ****
--- 346,361 ----
  	register char *p, *q;
  	char c;
  	char s[10];
+ 	char tmp_ptr[ 2 ];
+ 	char need_error_msg = 1;
  
  	if (argc != 3)
  		error("Usage: getopts optstring var");
+ 	p = lookupvar( "OPTIND" );
+ 	if ( ( p != NULL ) && ( *p == '1' ) && ( *( p + 1 ) == '\0' ) )
+ 		shellparam.optnext = NULL;
+ 	if ( argv[ 1 ][ 0 ] == ':' )
+ 		need_error_msg = 0;
  	if (shellparam.optnext == NULL) {
  		shellparam.optnext = shellparam.p;
  		shellparam.optptr = NULL;
***************
*** 369,375 ****
  	c = *p++;
  	for (q = argv[1] ; *q != c ; ) {
  		if (*q == '\0') {
! 			out1fmt("Illegal option -%c\n", c);
  			c = '?';
  			goto out;
  		}
--- 376,386 ----
  	c = *p++;
  	for (q = argv[1] ; *q != c ; ) {
  		if (*q == '\0') {
! 			tmp_ptr[ 0 ] = c;
! 			tmp_ptr[ 1 ] = '\0';
! 			setvar("OPTARG", tmp_ptr, 0);
! 			if ( need_error_msg )
! 				out1fmt("Illegal option -%c\n", c);
  			c = '?';
  			goto out;
  		}
***************
*** 378,391 ****
  	}
  	if (*++q == ':') {
  		if (*p == '\0' && (p = *shellparam.optnext++) == NULL) {
! 			out1fmt("No arg for -%c option\n", c);
! 			c = '?';
  			goto out;
  		}
  		setvar("OPTARG", p, 0);
  		p = NULL;
  	}
  out:
  	shellparam.optptr = p;
  	s[0] = c;
  	s[1] = '\0';
--- 389,409 ----
  	}
  	if (*++q == ':') {
  		if (*p == '\0' && (p = *shellparam.optnext++) == NULL) {
! 			shellparam.optnext--;
! 			tmp_ptr[ 0 ] = c;
! 			tmp_ptr[ 1 ] = '\0';
! 			setvar("OPTARG", tmp_ptr, 0);
! 			if ( need_error_msg )
! 				out1fmt("No arg for -%c option\n", c);
! 			c = ':';
  			goto out;
  		}
  		setvar("OPTARG", p, 0);
  		p = NULL;
  	}
  out:
+ 	fmtstr(s, 10, "%d", shellparam.optnext - shellparam.p + 1);
+ 	setvar("OPTIND", s, 0);
  	shellparam.optptr = p;
  	s[0] = c;
  	s[1] = '\0';

===========================================
-----
The static version's patch:
-----
===========================================
*** options.c.orig	Tue Oct 10 02:04:38 1995
--- options.c	Thu Jul 25 16:15:41 1996
***************
*** 346,355 ****
--- 346,362 ----
  	register char *p, *q;
  	char c;
  	char s[10];
+ 	char tmp_ptr[ 2 ];
+ 	static char need_error_msg = 1;
  
  	if (argc != 3)
  		error("Usage: getopts optstring var");
+ 	p = lookupvar( "OPTIND" );
+ 	if ( ( p != NULL ) && ( *p == '1' ) && ( *( p + 1 ) == '\0' ) )
+ 		shellparam.optnext = NULL;
  	if (shellparam.optnext == NULL) {
+ 		if ( argv[ 1 ][ 0 ] == ':' )
+ 			need_error_msg = 0;
  		shellparam.optnext = shellparam.p;
  		shellparam.optptr = NULL;
  	}
***************
*** 369,375 ****
  	c = *p++;
  	for (q = argv[1] ; *q != c ; ) {
  		if (*q == '\0') {
! 			out1fmt("Illegal option -%c\n", c);
  			c = '?';
  			goto out;
  		}
--- 376,386 ----
  	c = *p++;
  	for (q = argv[1] ; *q != c ; ) {
  		if (*q == '\0') {
! 			tmp_ptr[ 0 ] = c;
! 			tmp_ptr[ 1 ] = '\0';
! 			setvar("OPTARG", tmp_ptr, 0);
! 			if ( need_error_msg )
! 				out1fmt("Illegal option -%c\n", c);
  			c = '?';
  			goto out;
  		}
***************
*** 378,391 ****
  	}
  	if (*++q == ':') {
  		if (*p == '\0' && (p = *shellparam.optnext++) == NULL) {
! 			out1fmt("No arg for -%c option\n", c);
! 			c = '?';
  			goto out;
  		}
  		setvar("OPTARG", p, 0);
  		p = NULL;
  	}
  out:
  	shellparam.optptr = p;
  	s[0] = c;
  	s[1] = '\0';
--- 389,409 ----
  	}
  	if (*++q == ':') {
  		if (*p == '\0' && (p = *shellparam.optnext++) == NULL) {
! 			shellparam.optnext--;
! 			tmp_ptr[ 0 ] = c;
! 			tmp_ptr[ 1 ] = '\0';
! 			setvar("OPTARG", tmp_ptr, 0);
! 			if ( need_error_msg )
! 				out1fmt("No arg for -%c option\n", c);
! 			c = ':';
  			goto out;
  		}
  		setvar("OPTARG", p, 0);
  		p = NULL;
  	}
  out:
+ 	fmtstr(s, 10, "%d", shellparam.optnext - shellparam.p + 1);
+ 	setvar("OPTIND", s, 0);
  	shellparam.optptr = p;
  	s[0] = c;
  	s[1] = '\0';
>Audit-Trail:
>Unformatted:



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199607251418.QAA01059>