[Wylug-help] C++ code

Philip Wyett philipwyett at dsl.pipex.com
29 Oct 2002 00:52:10 +0000


--
On Sun, 2002-10-27 at 23:33, Roger Leigh wrote:
> Roger Greenwood <rg@nthong.freeserve.co.uk> writes:
>
> > The problem is buffers (i think). The code snippet following gets a
> > number from stdin, by way of any string entry. The commented out bits
> > have been left in so you can see where I am.
> >
> > What happens is that the number entered seems to leave an entry in the
> > buffer so that the next question is already answered (incorrectly).
>
> The buffer isn't cleared when you have read from it, which shouldn't
> be a problem.  However, I've seen this in the past, but can't remember
> the reason (I think it's the line feed at the end of the string).
> I've attached the code I use in gimp-print (for the escputil tool) at
> the end of the message.  This code has fully tested and working
> fgets() and readline() modes, which are fully commented (it's fairly
> obvious which parts use fgets()).  You'll also need to remove the _()
> i18n bits.  In case you wonder why input is static, that's so the
> caller doesn't need to care about memory allocation, but they can
> duplicate it if they need.
>
> I would suggest using libreadline instead of gets/fgets, but you
> *must* have GPL licensed your code to link with it.
>
> > int getnum(void)
> > {
> > 	int num;
> > 	double numd;
> > 	char entry[80];
>         const char* entry = NULL;
>
> > 	int valid = FALSE;
> > 	int exit = FALSE;
> > 	double dummy = 0;
> > 	double fraction;
> > 	do
> > 	{
> > 		fgets(entry,5,stdin);
>
>                 while (!entry)
>                   entry = readline("Enter a real number: ");
>                 /* link with -lreadline; your code must be GPLed */
>
> > 		numd = atof(entry);
> > 		fraction = modf(numd,&dummy);
>
>                 free (entry);
>                 entry = NULL;
>
> > 		if (fraction !=0)
>                    (fraction)
>
> > 		{
> > 			cout <<"\nEntry must be a whole number - decimals not allowed ";
> > 			numd = -1;
> > 			cout <<"\n\nPlease try again :- ";
> > 		}
> > 		if (numd == 0) exit = TRUE;
>                 if (!numd)
>                    exit = 1;
>
> > 		if (numd >0)
> > 		{
> > 			if (numd <= MAXINT)
> > 			{
> > 				valid = TRUE;
>                                 valid = 1;
>
> > 			}
> > 			else
> > 			{
> > 				cout << "\nEntry must be less than "<< MAXINT;
> > 				cout << "\n\nPlease try again :- ";
>                                 /* not valid C! */
>
> > 			}
> > 		}
> > 	}
> > 	while ((valid == FALSE) && (exit == FALSE));
>                (!valid && !exit)
>
> > 	if (exit == TRUE)
>            (exit)
>
> > 	{
> > 		num = 0;
> > 	}
> > 	else
> > 	{
> > 		num = int(numd);
>                       (int) numd; /* no int() function call! */
>
> > 	}
> > 	return(num);
>         return num; /* return is not a function call */
>
> > }
>
> >From an aesthetic point-of-view, I don't like the TRUE and FALSE
> macros; just use if (valid) or if (!valid) which are much safer and
> easier to read (if TRUE is 0, FALSE is !0, not 1 and vice-versa).  The
> definition of true and false can also change:
>
> if (exit)
>   exit(EXIT_SUCCESS);
> Here, exit is 1, which is FALSE!!  From the code above, I guess you
> used the opposite values, which is /not/ obvious!
>
> I would also check the return type of /every/ libc function (except
> perhaps printf) and print a diagnostic error message if it returns a
> failure:
>
> fprintf(stderr, "%s: Foo happened: %s\n", argv[0], strerror(errno));
> (If you only use GNU ld/GNU libc, you can use program_invocation_name
> and program_invocation_short_name instead of argv[0].)
>
> HTH,
> Roger
>
>
> ----do_get_input()----
> char *
> do_get_input (const char *prompt)
> {
>         static char *input = NULL;
> #if (HAVE_LIBREADLINE == 0 || !defined HAVE_READLINE_READLINE_H)
>         char *fgets_status;
> #endif
>         /* free only if previously allocated */
>         if (input)
>         {
>                 free (input);
>                 input = NULL;
>         }
> #if (HAVE_LIBREADLINE > 0 && defined HAVE_READLINE_READLINE_H)
>         /* get input with libreadline, if present */
>         input = readline ((char *) prompt);
>         /* if input, add to history list */
> #ifdef HAVE_READLINE_HISTORY_H
>         if (input && *input)
>         {
>                 add_history (input);
>         }
> #endif
> #else
>         /* no libreadline; use fgets instead */
>         input = xmalloc (sizeof (char) * BUFSIZ);
>         memset(input, 0, BUFSIZ);
>         printf ("%s", prompt);
>         fgets_status = fgets (input, BUFSIZ, stdin);
>         if (fgets_status == NULL)
>         {
>                 fprintf (stderr, _("Error in input\n"));
>                 return (NULL);
>         }
>         else if (strlen (input) == 1 && input[0] == '\n')
>         {
>                 /* user just hit enter: empty input buffer */
>                 /* remove line feed */
>                 input[0] = '\0';
>         }
>         else
>         {
>                 /* remove line feed */
>                 input[strlen (input) - 1] = '\0';
>         }
> #endif
>         return (input);
> }
> ----end do_get_input()----
>
>
> --
> Roger Leigh
>                 "Liberty and Livelihood" - Support the Countryside Alliance
>                 Need Epson Stylus Utilities? http://gimp-print.sourceforge.net/
>                 GPG Public Key: 0x25BFB848 available on public keyservers
>
> _______________________________________________
> Wylug-help mailing list
> Wylug-help@wylug.org.uk
> http://list.wylug.org.uk/mailman/listinfo/wylug-help

> > 		num = int(numd);
>                       (int) numd; /* no int() function call! */

Hi,

No int() function call? That is a valid way to enforce a type cast as
the method you used. both 'blah = (type)foo;' and 'blah = type(foo);'
are valid and comes with personal style preference. My preferred is the
method you employ, as it leaves no room for mis-interpretation.

In the above code is constants like EXIT_SUCCESS, BUFSIZE etc. If these
are define constants, they are dangerous. In small programs they are
fine, you can easily track them. However in larger projects, they often
become the source of bugs and are difficult too debug due too their
nature. The preprocessor merely replaces the word representation with
the value, not caring what it is aka the type or any relation too other
code.

An example is 'pi'. Many do:

#define pi 3.141592653589793

while

const float pi = 3.141592653589793;

is safer because the compiler and debugger are left in no doubt what it
is and can work with it. In the last project I was brought into. 15% of
the pre-release bug count was down to #define and some creative define
macros problems. I always encourage not using them wherever possible and
opting for the safer option IMHO.

Regards

Philip Wyett

--

Key pair can be obtained using the link below.

http://www.philipwyett.dsl.pipex.com/PhilipWyett.asc

--
--
Content-Description: This is a digitally signed message part

[ signature.asc of type application/pgp-signature deleted ]
--