Subject: Re: Possible bug? -- fixed

From: [email-masked]
Date: Wed, 10 May 2000 06:20:22 -0600

> > > >> Emulate foobar
> > > >> [FVWM][Emulate]: <<ERROR>> Unknown style ''
> > > >                                     ^^^^^
> > > >>
> > > >> *** FvwmDebug gives:
> > > >>
> > > >> [FVWM][Emulate]: <<ERROR>> Unknown style 'foobar'
> > > >> Unknown packet type 0x20000
> > > >  ^^^^^^^^^^^^^^^^^^^
> > >
> > > These are actually okay. The 'style' tokens are supposed to be
> > > present
> > > in the fvwm_msg() calls in the functions referenced.
> >

Hi, I've fixed my problem, it was in fvwm_msg(), I was reading
vsprintf's man page and came across this snipet.

-- nout = vsprintf(str,format,varlist);
-- ...
-- After a call to "vsprintf", the "varlist" variable should be assumed
-- to be in an undefined state. If you want to use "varlist" again, you
-- must call "va_end" to clean up, then "va_start" to reinitialize it.

anyways this psuedo patch fixes my problems. (mabey a va_end should be
added too?)

thanks,
sa

#### line 238 of misc.c
  else
  {
    fprintf(stderr,"[FVWM][%s]: %s ",id,typestr);
  }
  vfprintf(stderr, msg, args);
  fprintf(stderr,"\n");

+  va_start(args,msg);
+
  if (type == ERR)
  {
    /* I hate to use a fixed length but this will do for now */
    char tmp[2 * MAX_TOKEN_LENGTH];
    sprintf(tmp,"[FVWM][%s]: %s ",id,typestr);
    vsprintf(tmp+strlen(tmp), msg, args);
    tmp[strlen(tmp)+1]='\0';
    tmp[strlen(tmp)]='\n';
    if (strlen(tmp) >= MAX_MODULE_INPUT_TEXT_LEN)
      sprintf(tmp + MAX_MODULE_INPUT_TEXT_LEN - 5, "...\n");
    BroadcastName(M_ERROR,0,0,0,tmp);
  }

  va_end(args);
} /* fvwm_msg */

--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Dominik Vogt <[email-masked]>
Date: Wed, 10 May 2000 13:58:39 +0200

> -- must call "va_end" to clean up, then "va_start" to reinitialize it.

Uh, oh, I guess we'll have to use configure the v*printf functions
and provide a private replacement then?

Bye

Dominik ^_^

-- 
Dominik Vogt, Agilent Technologies, Dept. BVS
Herrenberger Str.130, 71034 Boeblingen, Germany
phone: 07031/464-4596, fax: 07031/464-3883, [email-masked]
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: [email-masked] (Paul D. Smith)
Date: 10 May 2000 10:43:06 -0400

>> -- must call "va_end" to clean up, then "va_start" to reinitialize it.

  dv> Uh, oh, I guess we'll have to use configure the v*printf functions
  dv> and provide a private replacement then?

I don't think so.  I think this must be assumed for all types of
functions that take variadic arguments.  At least, my close reading of
the Solaris man page for vsprintf, while it's not so blunt, seems to
imply the same.

Certainly it's true in the C standard that once you walk through
argument lists with va_arg(), you must run va_end() and then va_start()
again before you can restart the walk.  It's not clear (to me) from the
standard whether that holds true if you invoke a function that calls the
va_arg() for you, but I see no reason to assume that it isn't.  For
example, the type of va_list is opaque and could _very_ well be a pointer
to a structure; then passing that pointer to a sub-function which uses
va_arg() would change the contents of the structure for the parent
function as well.

Anyway, it certainly can't hurt anything to program defensively here
even in situations where it's not necessary, so we can just do that; we
don't need configure checks for this (writing that--bleah! :).

E.g., this patch.  I can commit it, unless I missed something.

2000-05-10  Paul D. Smith  <[email-masked]>

	* fvwm/misc.c (fvwm_msg): Wrap _each_ instance of vsprintf() in
	va_start()/va_end().

Index: fvwm/misc.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/misc.c,v
retrieving revision 1.87
diff -u -B -b -r1.87 misc.c
--- fvwm/misc.c	2000/02/11 16:14:59	1.87
+++ fvwm/misc.c	2000/05/10 14:40:56
@@ -311,8 +311,6 @@
       break;
   }
 
-  va_start(args,msg);
-
   if (Scr.NumberOfScreens > 1)
   {
     fprintf(stderr,"[FVWM.%d][%s]: %s ",(int)Scr.screen,id,typestr);
@@ -321,7 +319,11 @@
   {
     fprintf(stderr,"[FVWM][%s]: %s ",id,typestr);
   }
+
+  va_start(args,msg);
   vfprintf(stderr, msg, args);
+  va_end(args,msg);
+
   fprintf(stderr,"\n");
 
   if (type == ERR)
@@ -329,7 +331,9 @@
+    va_end(args,msg);
@@ -337,7 +341,6 @@
 
-  va_end(args);
 } /* fvwm_msg */
 #endif

---

From: Dominik Vogt <[email-masked]>
Date: Wed, 10 May 2000 16:33:46 +0200

> the Solaris man page for vsprintf, while it's not so blunt, seems to

I read the Linux man page and I can find nothing implying this
limitation (but perhaps I don't read it carefully). (man page
attached).

> don't need configure checks for this

> (writing that--bleah! :).

8-) I thought so.

> E.g., this patch.  I can commit it, unless I missed something.

Go ahead.  There are several more files that use v*printf.
I didn't check if they need patching, though.

libs/debug.c
modules/FvwmIconMan/debug.c
modules/FvwmWinList/FvwmWinList.c
vms/vms.c

Bye

Dominik ^_^

---

From: Dominik Vogt <[email-masked]>
Date: Wed, 10 May 2000 16:34:53 +0200

... and now the attached man page :-)

---

From: [email-masked] (Paul D. Smith)
Date: 10 May 2000 12:12:44 -0400

dv> I read the Linux man page and I can find nothing implying this
  dv> limitation (but perhaps I don't read it carefully). (man page
  dv> attached).

The Linux printf() man page says almost nothing about the v* versions at
all.  I don't think it's a limitation, per se.  You should check in the
man page for va_start/va_arg/va_end; that man page will _clearly_ state
that to re-walk the arguments after walking them once, you must va_end,
then va_start again.

<flip flip flip> Ah.  I just checked the C standard, and the rule is
actually quite well-defined about this.  I should have done so in the
first place.

Section 4.8 "Variable Arguments <stdarg.h>" (p 123, lines 11-15)

  ...
    If access to the varying arguments is desired, the called function
    shall declare an object (referred to as <ap> in this section) having
    type <va_list>.  The object <ap> may be passed as an argument to
    another function; if that function invokes the <va_arg> macro with
    parameter <ap>, the value of <ap> in the calling function is
    indeterminate and shall be passed to the <va_end> macro prior to any
    further reference to <ap>.
  ...

Pretty clear :).  Note this is the original ANSI C standard, not the
renumbered ISO version.

  dv> Go ahead.  There are several more files that use v*printf.
  dv> I didn't check if they need patching, though.

The only places where there are problems is when you walk the arg list
_twice_.  Then you need to end/re-start between the two walks.

  dv> libs/debug.c
  dv> modules/FvwmIconMan/debug.c
  dv> modules/FvwmWinList/FvwmWinList.c
  dv> vms/vms.c

Thanks, I'll check these, too.

-- 
-------------------------------------------------------------------------------
 Paul D. Smith <[email-masked]>         Network Management Development
 "Please remain calm...I may be mad, but I am a professional." --Mad Scientist
-------------------------------------------------------------------------------
   These are my opinions---Nortel Networks takes no responsibility for them.
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked]

---

From: Chris Rankin <[email-masked]>
Date: Thu, 11 May 2000 08:23:28 +1000 (EST)

>     further reference to <ap>.

All very well, but what if you're passing va_list arguments around as
function parameters until finally printing them out with vfprintf()?

eg

void
foo(const char *msg, va_list ap)
{
  /* walk the argument list in vfprintf */
  vfprintf(file1, msg, ap);

  /* this is now probably illegal!! */
  vfprintf(file2, msg, ap);
}

void
print_message(const char *msg, ...)
{
  va_list  ap;
  va_start(msg, ap);
  foo(msg, ap);
  va_end(ap);
}

I don't see any way of calling va_start again from within foo(). Looks
like a minefield.

Chris.

---

From: [email-masked] (Paul D. Smith)
Date: 10 May 2000 19:50:17 -0400

>> further reference to <ap>.

  cr> All very well, but what if you're passing va_list arguments around as
  cr> function parameters until finally printing them out with vfprintf()?

  cr> void
  cr> foo(const char *msg, va_list ap)
  cr> {
  cr>   /* walk the argument list in vfprintf */
  cr>   vfprintf(file1, msg, ap);

  cr>   /* this is now probably illegal!! */

Definitely illegal.

  cr>   vfprintf(file2, msg, ap);
  cr> }

  cr> void
  cr> print_message(const char *msg, ...)
  cr> {
  cr>   va_list  ap;
  cr>   va_start(msg, ap);
  cr>   foo(msg, ap);
  cr>   va_end(ap);
  cr> }

  cr> I don't see any way of calling va_start again from within foo().

You're right, you can't (portably).

  cr> Looks like a minefield.

Well, as the doctor said, "just don't do that!" :).