Thread: 2 very noob questions

There's a truckload of stuff wrong with this! Here we go...

Code:

#include <stdio.h>
#include <string.h>

/*Need to include stdlib.h and conio.h*/

#define firstnames 15	/*Style issue - Macros are normally capitalized*/
#define lastnames 15	/*to differentiate from normal variables*/
#define names 30

int main ()
{
 char firstname[firstnames];
 char lastname[lastnames];
 char name[names];
 char *string;
 int x;	
 char y = '!';

 string	= name;		/*Why are you doing this? You could just use name?*/

 printf("What yo first name be son?\n");
 gets(firstname);		/*Don't use gets - use fgets*/
 if(gets == 0x00)		/*Not the best way to check return of a function*/
 {
  printf("you know yo name ain't dat long.\n");
  exit();	/*exit() - defined in stdlib.h - needs a parameter to return to OS*/
 }
 strcpy(name, firstname);
 printf("and yo last name?\n");
 gets(lastname);
 if(gets == 0x00)
 {
  printf("you know yo name ain't dat long.\n");
  exit();
 }
 strcpy(name, lastname);
 printf("hmm. so this be yo name then?");
 while(y != '\0')	        /*You never change the value of y - */
 {				/*this should loop infinitely*/
  printf("%C", string[x]);	/*x is uninitialised before this use - */
 }				/*resulting in undefined behavior*/
 getch();	/*Defined in conio.h*/
}

/*also - the format specifier is %c not %C*/

and now for the fix... This is a trimmed down version of your
program - I'm not sure of what type of error checking you
were trying to use, so i removed it. Also, no need for getch ()
IMO - The console can be kept open using getchar ();

Code:

#include <stdio.h>
#include <string.h>

#define FIRST 15
#define LAST 15
#define FULL 30

int main ()
{
 char firstname[FIRST];
 char lastname[LAST];
 char name[FULL];

 char *ptr;

 printf("What yo first name be son?\n");
 fgets(firstname, FIRST, stdin);

 if ((ptr = strchr(firstname, '\n')) != NULL)
	 *ptr = '\0';
 
 strcpy(name, firstname);

 printf("and yo last name?\n");

 fgets(lastname, LAST, stdin);
 
 if ((ptr = strchr(lastname, '\n')) != NULL)
	 *ptr = '\0';

 strcat(name, lastname);

 printf("hmm. so this be yo name then?\n");
 printf ("%s", name);

 getchar();
 return 0;
}

Read this on fgets
Also important - Why gets is bad

There are quite a few errors her so bear with me.

Never use gets. LINK Replace it with fgets. LINK

if(gets == 0x00)

What is this??? What is gets supposed to be? I think that you want to check the string that you just got with gets. That would just be

Code:

if( firstname[0] == '\0')

That would check to see if the first character was a null character symbolizing the end of the string.

Code:

strcpy(name, lastname);

strcpy is just putting lastname over the previous contents of name. You want strcat. strcat adds the second argument to the end of the first one. So you would use the same to arguments as you used in strcpy.

Code:

while(y != '\0')
 {
  printf("%C", string[x]);
 }

y is initialized as an exclamation point(!). Therefore '!' will never be '\0'. This is an infinite loop. And the

Code:

string[x]

does not do what you want it to. x has not yet been initialized. This is just taking any random number and trying to access that number's spot in string. There is a very good chance that this is way out of the array's bounds. I think that you wanted

Code:

x = 0;                      /*Set x to 0 so that it will start at the beginning of the array*/
while( string[x] != '\0' )  /*While the current spot in string is not the end of the string*/
{
     putchar( string[x] );  /*Print out the current letter*/
     x++;                   /*Increment x by 1 so that it will move up to the next spot in the array*/
}

There is no need to use getch at the end. You can just replace this with getchar. If you do use getch you would need to include conio.h which is not a standard header and not all compilers have the same version of this.


The defines do not do much. You are only using the numbers once so it is not necessary but it can be desired for easier reading. It is not a rule but it is common practice to make your defines all caps. so firstnames would be FIRSTNAMES or something like that. Also you have two defines that both are the same thing so if you wanted to you could make them both NAMELEN.


exit is not necessary. Exit is used to return a number to the OS. exit(0) is the same as return 0;. If you do use exit you need to include stdlib.h.


This would make the final code:

Code:

#include <stdio.h>
#include <string.h>

#define NAMELEN 15
#define FULLNAMELEN 30

int main ()
{
 char firstname[NAMELEN];
 char lastname[NAMELEN];
 char name[FULLNAMELEN];
 char *string;
 int x;
 char y = '!';

 string = name;

 printf("What yo first name be son?\n");
 fgets(firstname, sizeof(firstname), stdin);
 if(firstname[0] == '\0')
 {
  printf("you know yo name ain't dat long.\n");
  return 1;
 }
 strcpy(name, firstname);
 printf("and yo last name?\n");
 fgets(lastname, sizeof(lastname), stdin);
 if(lastname[0] =='\0')
 {
  printf("you know yo name ain't dat long.\n");
  return 1;
 }
 strcpy(name, lastname);
 printf("hmm. so this be yo name then?");
 while(string[x] != '\0')
 {
  putchar(string[x]);
 }
 getchar();
}

I hope that this helps!.
~Sven

Originally Posted by chasingxsuns

Code:

if ((ptr = strchr(firstname, '\n')) != NULL)
	 *ptr = '\0';

can you explain what this does?i cant figure it out lol.

strchr is a function for splitting up a string. It takes a string and a character to look for. When it finds thie character it returns a pointer to that first part of the string. If it does not find that character in the string it returns NULL. So this call to strchr would search firstname for the character '\n'. The if statement means if there is no '\n' in firstname it will make the contents of ptr '\0'.
~Sven

fgets is a fantastic function in that it prevents buffer overflows,
but it can store the newline character if the string entered is
less than the length of the second parameter (the container size).
strchr is a function that searches a string for a specific character
and returns the memory address of where it found that character
in the string, or NULL if it can't be found. Then, this line

*ptr = '\0';

overwrites that memory location returned by strchr with the
string termination character (\0). Easy Peasy!

The way fgets works, is that it reads from the input buffer one
character at a time, untill the container is filled. If too many
characters are entered, they remain in the input buffer. The
result is that after a call to fgets, the input buffer MAY need to
be flushed. The easiest way to do this is with getchar, but
if there was no need to flush the buffer, the program will stall
and wait for an input. To that end, its a good idea to write
a function that you can call just like fgets, removes the newline
character if necessary, and also flushes the input buffer if
necessary - you might want something like this as a result:

Code:

void strget (char *filename, int size, FILE *ip)
{
	char *p;
	int clear;

	fgets (filename, size, ip);

	if ((p = strchr(filename, '\n')) != NULL)
		*p = '\0';

	else while ((clear = getchar ()) != '\n' && (clear != EOF));
}

You can always also just do

Code:

filename[strlen(filename)-1] = '\0'

This takes the number of characters in the string and then subtracts one (because arrays start at 0 and not 1) and if the string was gotten with fgets ithis character will be a '\n'. It then replaces that character with a '\0'.
~Sven

not the best method Sven, what if exactly the right amount or
more characters are entered? that would knock off the last
character of the string. a better approach is this:

Code:

if (buffer[strlen(buffer-1)] == '\n')
    buffer[strlen(buffer-1)] = '\0';

You need to add the -1 after strlen(buffer). Arrays start at 0 and not one. Buffer[strlen(buffer)] is out of bounds.
~Sven

correct you are Sven, i've editted my post.

As for the new problem, when the user enters too many
characters, they are left in the input buffer, so the call to getchar
automatically gets a character and the program closes.

What you need to do is to literally check if too many characters
are entered. If too many are entered, the value at
string[strlen(string)-1] won't be '\n' - use an if statement to
check this, and if it is not true, flush the buffer and call getchar
again:

Code:

if (string[strlen(string)-2] != '\n')
    while ((clear = getchar ()) != '\n' && (clear != EOF));
    printf ("Your message\n");
    getchar();

NOTE: this will only be accurate if you haven't already removed
the newline - do that after this code is called.

Originally Posted by 00Sven

You need to add the -1 after strlen(buffer). Arrays start at 0 and not one. Buffer[strlen(buffer)] is out of bounds.
~Sven

No it isn't. It points to the null character. strlen returns the length of the string, not including the null.


Quzah.