Mail Archives: djgpp-workers/2003/01/05/11:26:45
Hello.
ams AT ludd DOT luth DOT se wrote:
[snip]
> > What happens to the multiplier? It doesn't seem to be used.
>
> It's a verification that the value is sane. (stubedit doesn't do this.)
OK.
> > Shouldn't the format be %lu%c, since stack_size is an unsigned long?
>
> Yes.
>
> > > + {
> > > + v_printf("Using " MINSTACK_ENV_VAR " = %s.\n", env_stack_size);
> > > +
> > > + stubedit_str = malloc( sizeof("stubify ")+strlen(ofilename)+sizeof(" minstack=")+strlen(env_stack_size)+1 );
> >
> > Why do you do sizeof("stubify ")? Shouldn't it be strlen? And on "stubedit "?
> > I.e.: strlen("stubedit ")? As it turns out, the number from the above code is
> > right.
>
> As strlen("some string") == sizeof("some string") in C, that shouldn't
> be a surprise. It's for removing an unnecessary function call when we
> can calculate it statically.
OK, but shouldn't it be sizeof("stubedit "), since that's what gets written
into the string? You could also remove the "+1" at the end, since sizeof will
include the nul terminator.
> Note 1. Perhaps gcc is smart enough to do this optimisation anyway. (I
> don't care.)
>
> Note 2. Perhaps some people don't like it. (I care.)
>
> Let me know if you oppose violently.
No, not at all! 8) I was just a bit concerned by the use of stubify instead of
stubedit in the string.
> > > + {
> > > + fprintf(stderr, "%s: illegal minimum stack size specified, check environment variable " MINSTACK_ENV_VAR "\n", ofilename);
> >
> > You could display the contents of the environment variable here.
>
> Hmm. Suggest wording/layout. In particular ``printf("check environment
> variable " MINSTACK_ENV_VAR "=%s\n, env_stack_size)'' is incorrect. Or
> am I just too sensitive?
How about:
fprintf(stderr, "%s: illegal minimum stack size specified in environment
variable " MINSTACK_ENV_VAR ": %s\n", ofilename, env_stack_size);
> > Apart from those comments, it looks good to me. Although I hope the code
> > is readable without line wrapping in 80 columns.
>
> Well, it not unreadable for _me_. I see now that I look, that the
> earlier code limited itself to ~80 columns. I'll do something.
It wasn't unreadable for me, either. I thought it was one of the
generally-accepted coding style rules that code should be limited to within 80
columns. (I try 74, so that diffs look OK when e-mailed.) OK, maybe I didn't
say what I meant.
But I learned a few years ago that it's pointless to get in a coding style
war. I always try to go with the "local style", whatever that is (GNU, K&R).
Bye, Rich =]
--
Richard Dawe [ http://www.phekda.freeserve.co.uk/richdawe/ ]
- Raw text -