[Basic C] Inventory Editing

[Strydhaizer]

#include <conio.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
//Inventory.txt
struct prodrec 
	{
	char prodcode[5];
	char prodname[10];
	int prodcount;
	};

main()
{
	FILE *s;
	s = fopen("Inventory.txt", "w"); 
	
	int prodquant;
	char cont, cont2, operand;
	int ctr, e;
	
	char prodcodeupd[2]; //Updating Values
	char produpd[1];
	int prodcountupd=0;
	
	
			printf ("How many products: ");		//Inventory Input
			scanf ("%d", &prodquant);
			struct prodrec prod[prodquant];
main:
				for (ctr=0; ctr<prodquant; ctr++)
					{
					printf ("nProduct Code: ");
					scanf ("n");
					gets (prod[ctr].prodcode);
					printf ("Product Name: ");
					scanf ("n");
					gets (prod[ctr].prodname);
					printf ("Quantity: ");
					scanf ("%d", &prod[ctr].prodcount);
					}
				printf ("nProduct CodetProduct NametQuantity");
				for(ctr=0;ctr<prodquant;ctr++)
					{
					printf("n%stt%stt%d", prod[ctr].prodcode, prod[ctr].prodname, prod[ctr].prodcount);
					}
		printf("nDo you want to Update the Inventory [Y/N]:");
		scanf ("n%c", &cont);
	
		if (cont=='N'||cont=='n')
				{
mainend:
						printf ("nProduct CodetProduct NametQuantity");
						for(ctr=0;ctr<prodquant;ctr++)
							{
							printf("n%stt%stt%d", prod[ctr].prodcode, prod[ctr].prodname, prod[ctr].prodcount);
							fprintf(s, "%stt%stt%d", prod[ctr].prodcode, prod[ctr].prodname, prod[ctr].prodcount);
							}
						system ("exit");
				}	
		
		else if (cont=='Y'||cont=='y')
		{
update:
		printf("nProduct Code: ");
		scanf("n");
		gets (prodcodeupd);
	
		for(ctr=0; ctr<prodquant; ctr++)
			{	
			if (strcmp(prodcodeupd, prod[ctr].prodcode)==0)
				{
				printf ("Update Code[A=Add/S=Subtract]: " );
				scanf("%c", &operand);
					if (operand=='A'|| operand=='a')
						{
						printf ("Quantity: ");
						scanf("%d", &e);
						prod[ctr].prodcount+=e;
						}
					else if (operand=='S'|| operand=='s')
						{
						printf ("Quantity: ");
						scanf("%d", &e);
						prod[ctr].prodcount-=e;
						}

				}
			
			else if (strcmp(prodcodeupd, prod[ctr].prodcode)!=0)
				{
				printf ("Record not foundnn");
				}
				
			printf("nUpdate More [Y/N]: ");
			scanf ("n%c", &cont2);
				if (cont2=='Y'||cont2=='y')
					{
					goto update;
					}
				else
					{
					goto mainend;
					}
		
		
			}
		}
		
		
		/**/
	fclose(s); 
	getch();
}

Now the problem is it can't find the second product code.

Click

Any help will be appreciated :').

 

[Dr Super Good]

char prodcodeupd[2]; //Updating Values

You only allocate space for 2 characters yet product code strings require at least 3 character storage. C strings are "null" terminated meaning that an extra invisible character is required at the end to signify the end of string. A string consisting of 2 readable characters requires 3 character spaces to store due to this extra null terminator.

More about null terminated strings.

As such when you get the product code string you are suffering a buffer overflow error where it will modify memory outside of its allotted space. This could be written in unallocated parts of the stack, automatically allocated parts of the stack or modifying space allocated for other variables, all of which are very bad. If it works for any product at all is probably down to luck. Errors such as this are loved by hackers as they are great for creating malware with.

I would advise using more safe/powerful string operations. Consider dynamically allocating string space as required rather than using static buffers. If using C++ you can use their standard string class to do a lot of the grunt work for you.

Also do not assume 1 character is 1 "char" in size. Many encodings such as UTF-8 use variable length characters.

 

[Strydhaizer]

As such when you get the product code string you are suffering a buffer overflow error where it will modify memory outside of its allotted space. This could be written in unallocated parts of the stack, automatically allocated parts of the stack or modifying space allocated for other variables, all of which are very bad. If it works for any product at all is probably down to luck. Errors such as this are loved by hackers as they are great for creating malware with.

I would advise using more safe/powerful string operations. Consider dynamically allocating string space as required rather than using static buffers.

Also do not assume 1 character is 1 "char" in size. Many encodings such as UTF-8 use variable length characters.

Well im sure i did'nt understand the rest, but anyway, thanks.

If using C++ you can use their standard string class to do a lot of the grunt work for you.

Nah sadly im just starting to learn it.

 

[Dr Super Good]

Well im sure i did'nt understand the rest, but anyway, thanks.

Did raising the buffer size fix the problem or not?

 

[Strydhaizer]

Did raising the buffer size fix the problem or not?

No, i did'nt touch anything anymore, i just removed the "printf ("Record not foundnn")" thing.

Did raising the buffer size fix the problem or not?

I am new to this thing and i dont know what the buffer size is and what's the relation of it on my codes.

Anyway, as far as i remember i am just asking for some logic fixes, you're mentioning so many things which i still dont know right now unless you give me another year for me to study it .

 

[Dr Super Good]

I am new to this thing and i dont know what the buffer size is and what's the relation of it on my codes.

A buffer.
In your case...

    char prodcodeupd[2]; //Updating Values

This buffer can hold a string at most 1 char long. This is because 1 char is needed for the null string terminator and 2 (buffer length) - 1 (null terminator length) = 1 (maximum string length). Since the product codes are at least 2 char long one can see the buffer is not big enough to hold the product code strings.

It is very likely that it is overwriting some other area of memory as C arrays do not pass size information around at runtime. It is also possible the compiler is adding hidden safety to your code though by limiting the string length to 1 with a special implementation of the function as the compiler does know the array length at compile time, after all buffer overrun errors are one of the most common exploits used maliciously by hackers.

Raising it to something like...

    char prodcodeupd[16]; //Updating Values

Should hopefully fix the problem as now a string up to 15 char long can be stored.

Do note that 1 char may not necessarily be 1 character. Depending on encoding, eg UTF-8, a character can end up more than 1 char long. This is not usually the case for common English characters, but is especially the case for glyph based characters such as used by Asia.

 

[GhostWolf]

An array of two chars can hold two chars. If you write something to the second char, you are not overwriting any other memory, since it's memory you allocated...
What CAN mess up is standard operations on strings that expect a null terminated string (e.g. printf).
The actual issue, which would have made your logical problem obvious, is that you are not actually setting the last character to null at all. C doesn't do that for you, as far as I remember.

char bla[2]; // bla[0] and bla[1] hold arbitrary values at this point

bla[1] = 0; // null terminated string

 

[edo494]

Yes GH is right, in C when you dont write anything to variable when you define the variable, the real value stored inside that is unspecified, but if its array and you initialize even one element, it will init all rest to 0, so

char bla[2] = {0}; will work too, and imo is nicer and more compact

 

[Dr Super Good]

An array of two chars can hold two chars. If you write something to the second char, you are not overwriting any other memory, since it's memory you allocated...

Yes. However he is not writing characters, rather strings.

gets (prodcodeupd);

Get string from stdin

Reads characters from the standard input (stdin) and stores them as a C string into str until a newline character or the end-of-file is reached.

The newline character, if found, is not copied into str.

A terminating null character is automatically appended after the characters copied to str.

Notice that gets is quite different from fgets: not only gets uses stdin as source, but it does not include the ending newline character in the resulting string and does not allow to specify a maximum size for str (which can lead to buffer overflows).

Since he is inputting a 2 ASCII character string (1 char per character), it is writing 3 char into the buffer (the 2 character input, and 1 char null terminator). The buffer is only 2 char long hence it is placing 1 char outside the buffer, unless the compiler detects this and forces some sort of safety.

Maybe you are talking about a different problem? I mean I do not see uninitialized variable being a problem for that as he loads a string into the buffer array before using it. Just the string he loads into it is longer than the buffer.

if "gets" should be used is another question. Its inherit lack of safety generally makes it bad to use.