[Freeciv-Dev] Re: dynamic timeout (PR#1356)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Sun, Apr 07, 2002 at 10:45:31AM -0500, Mike Kaufman wrote:
> On Sun, Apr 07, 2002 at 02:50:40PM +0200, Raimar Falke wrote:
> > > +***************************************************************/
> > > +void get_args(char *str, char **args, int numargs)
> > 
> > Please pass in the maximum size.
> 
> I do not understand. str's maximum size? why? 
> numargs should be <= the size of the args array. I don't see why we need
> anything else. Please be explicit.

*rereading the patch* I misunderstood it. Neither the caller nor the
callee allocate memory but the args pointers point into the original
string. Uhhh. Such pointer aliasing should IMHO be avoided. This
construct

   char str[MAX_LEN];
   char *args[numargs];

   sz_strlcpy(str, buffer);
   get_args(str, args, numargs);

   str[0]='x';

will also change args[0][0]. This isn't intuitive.


> > > +{
> > > +  int i, len;
> > > +  bool quoted = FALSE;
> > 
> > What about single quoted. See cut_comment.
> 
> These are not analogous situations. cut_comment merely looks for paired
> sets before it gets to the octothorp. Here we've got to keep track of
> everything. Besides, I want to be able to create players like:
> create "ain't I cool" "don't mess with me" 
> which won't work if apostrophes are special. (of course we could expect
> the user to \', but now we expect the user to use double quotes, either
> way the user has some responsibility)

And want to allow a player name 'Joe "The Hammer" User'

> what would be relatively easy would be to treat ' and " interchangably.
> so "5' 'abc da' "4t" 'a s" returns 4 tokens. To actually treat ' and "
> in a nested fashion would suck ass. I don't have the energy (ok, it
> wouldn't be _that_ hard, but I still don't have the energy). I look forward 
> to your implementation. Extend this one after it's in cvs.

No problem adding it before it goes into CVS.

> > > +  bptr = buf;
> > > +
> > > +  /* NULL out the args */
> > > +  for(i = 0; i < numargs; i++) {
> > > +    args[i] = NULL;
> > > +  }
> > > +  i = 0;
> > > +
> > > +  while(*bptr != '\0' && i < numargs) {
> > > +    /* skip intervening whitespace */
> > > +    while(!quoted 
> > > +          && (*bptr == ' ' || *bptr == '\t' || *bptr == ',' || *bptr == 
> > > '"')) {
> > > +      sptr++; bptr++;
> > > +      if (*(bptr - 1) == '"') {
> > > +        quoted = TRUE; /* starting a quote */
> > > +        break;
> > > +      }
> > > +    }
> > > +
> > > +    args[i++] = sptr;
> > > +
> > > +    /* skip arg */
> > > +    while(*bptr != '\0'
> > > +          && ((*bptr != ' ' && *bptr != '\t' && *bptr != ',') || 
> > > quoted)) {
> > > +      if (*bptr == '"') {
> > > +        quoted ^= 1; /* could be starting or ending */
> > > +        break;
> > > +      }
> > > +      sptr++; bptr++;
> > > +    }
> > > +
> > > +    if (*bptr == '\0') {
> > > +      break;
> > > +    } else {
> > > +      *(sptr++) = '\0';
> > > +      bptr++;
> > > +    }
> > > +  }
> > > +
> > > +  free(buf);
> > > +}
> > > +
> > 
> > > +/**************************************************************************
> > > +  Set timeout options.
> > > +**************************************************************************/
> > > +static void timeout_command(struct connection *caller, char *str) 
> > > +{
> > > +  char buf[MAX_LEN_CONSOLE_LINE];
> > > +  char *arg[4];
> > > +  int i = 0, noargs = 0, val[4];
> > > +  int *timeouts[4];
> > > +
> > > +  timeouts[0] = &game.timeoutint;
> > > +  timeouts[1] = &game.timeoutintinc;
> > > +  timeouts[2] = &game.timeoutinc;
> > > +  timeouts[3] = &game.timeoutincmult;
> > > +
> > > +  sz_strlcpy(buf, str);
> > > +  get_args(buf, arg, 4);
> > > +
> > > +  for(i = 0; i < 4; i++) {
> > > +    if (arg[i] != NULL) { 
> > > +      if(sscanf(arg[i], "%d", &val[i]) != 1) {
> > > +        cmd_reply(CMD_TIMEOUT, caller, C_FAIL, _("Invalid argument 
> > > %d."), i+1);
> > > +      } else {
> > 
> > > +        *timeouts[i] = val[i];
> > 
> > Why do you first use val here and copy it later to timeouts?
> 
> 'cause we may get garbage on the sscanf and we want to protect the
> original value. But we should get rid of val[4] in favor of a tmp var inside 
> the for loop.

sscanf doesn't overwrite the parameter if no valid input is found (if
it returns 0). So the tmp variable isn't need.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "This is Linux Country. On a quiet night, you can hear Windows reboot."