Inventory program, possible boundary value issue?

Question

The below program seems to work well in most situations, but if I add a record for the last inventory position (record 10), it causes issues. Specifically, if I add record 10 then try and delete it, it still shows in the inventory after I call printList( ). This is only the case for the final record, and doesn't occur for any of the others.

Can anyone work out what the issue is? I've been using -Wall when using gcc to compile, and it isn't issuing any warnings. I've also been trying to work out how to use gdb, but I'm still learning, so that hasn't been helpful either.

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

typedef struct {
  unsigned int record;
  char tool[30];
  unsigned int quantity;
  double price;
} hardware;

void menu(FILE *fPtr, hardware *toolsPtr);
void initialiseRecords(FILE * fPtr, hardware *toolsPtr);
void inputTool(FILE * fPtr, hardware *toolsPtr);
void printList(FILE * fPtr, hardware *toolsPtr);
void deleteRecord(FILE * fPtr, hardware *toolsPtr);

int main(void)
{
  
  FILE *fPtr;
  hardware tools = { 0, "", 0, 0.0 }, *toolsPtr;
  toolsPtr = &tools;

  if ((fPtr = fopen("hardware.dat", "wb+")) == NULL) {
    puts("File cannot be opened.") ;
  }
  else {
    menu(fPtr, toolsPtr);
  }

  fclose(fPtr);
}

void menu(FILE *fPtr, hardware *toolsPtr)
{
  unsigned int choice;
  
  printf("\n%s\n\n%s\n\n%s\n%s\n%s\n%s\n%s\n\n%s",
         "** Hardware Inventory Program **", 
         "    Enter a menu option:",
         "1 - Initialise the record file.",
         "2 - Add a record to the file.",
         "3 - Delete a record from the file.",
         "4 - Print the current inventory",
         "5 - Quit the program.", "> ");

  scanf("%u", &choice);

  while (choice != 5) {

    switch (choice) {
    case 1:
      initialiseRecords(fPtr, toolsPtr);
      break;
    case 2:
      inputTool(fPtr, toolsPtr);
      break;
    case 3:
      deleteRecord(fPtr, toolsPtr);
      break;
    case 4:
      printList(fPtr, toolsPtr);
      break;
    default:
      puts("Incorrect entry.");
      break;
    }

    printf("\n%s\n\n%s\n\n%s\n%s\n%s\n%s\n%s\n\n%s",
           "** Hardware Inventory Program **", 
           "    Enter a menu option:",
           "1 - Initialise the record file.",
           "2 - Add a record to the file.",
           "3 - Delete a record from the file.",
           "4 - Print the current inventory",
           "5 - Quit the program.", "> ");

    scanf("%u", &choice);
  }
}

void initialiseRecords(FILE * fPtr, hardware *toolsPtr)
{
  fseek(fPtr, 0, SEEK_SET);

  for (unsigned int i = 0; i < 10; ++i) {

    char s[30] = "";
    sscanf(s, "%s", toolsPtr->tool);

    toolsPtr->record = i + 1;
    toolsPtr->quantity = 0;
    toolsPtr->price = 0.0;

    fwrite(toolsPtr, sizeof(hardware), 1, fPtr);
  }
}

void inputTool(FILE * fPtr, hardware *toolsPtr)
{
  printf("\n%s\n\n%s", "Enter record # (-1 to quit):", "> ");
  scanf("%u", &toolsPtr->record);

  while(toolsPtr->record != -1) {

    fseek(fPtr, (toolsPtr->record - 1) * sizeof(hardware), SEEK_SET);
    fread(toolsPtr, sizeof(hardware), 1, fPtr);
    getchar();

    if (!strcmp(toolsPtr->tool, "")) {

      printf("\n%s\n\n%s", "Enter tool name:", "> ");
      fgets(toolsPtr->tool, 30, stdin);

      toolsPtr->tool[strlen(toolsPtr->tool) - 1] = '\0';

      printf("\n%s\n\n%s", "Enter quantity:", "> ");
      scanf("%u", &toolsPtr->quantity);

      printf("\n%s\n\n%s", "Enter price:", "> ");
      scanf("%lf", &toolsPtr->price);

      fseek(fPtr, (toolsPtr->record - 1) * sizeof(hardware), SEEK_SET);
      fwrite(toolsPtr, sizeof(hardware), 1, fPtr);
    }
    else {
      getchar();
      puts("There is an existing record with this number.");
      fseek(fPtr, 0, SEEK_SET);
    }
    printf("\n%s\n\n%s", "Enter record (-1 to quit):", "> ");
    scanf("%u", &toolsPtr->record);
  }
}

void printList(FILE * fPtr, hardware *toolsPtr)
{
  fseek(fPtr, 0, SEEK_SET);
  printf("\n%-10s%-30s%-10s%-10s\n\n", "Record #", 
             "Tool Name", "Quantity", "Price");

  for (unsigned int i = 0; i < 10; ++i) {

    fread(toolsPtr, sizeof(hardware), 1, fPtr);
    printf("%-10u%-30s%-10u$%-10.2lf\n",
           toolsPtr->record, toolsPtr->tool, 
           toolsPtr->quantity, toolsPtr->price);
  }
}

void deleteRecord(FILE * fPtr, hardware *toolsPtr)
{
  printf("\n%s\n\n%s", 
             "Enter the record number of the tool to delete:", "> ");
  scanf("%u", &toolsPtr->record);

  fseek(fPtr, (toolsPtr->record - 1) * sizeof(hardware), SEEK_SET);

  fwrite(toolsPtr, sizeof(hardware), 1, fPtr);
}

Bob Shaffer · Answer 1 · 2019-02-04T02:31:29.970

0

Basically, using stdio.h functions, you cannot really delete anything. There isn't even an ftruncate (although I do have such a function in unistd.h). So, when working with this type of database, you really should do this:

Add a "deleted" field to each record. Use it to determine which records should be shown.

Add an "id" field to each record. That way the same record always has the same key.

When you are adding a record, look for a deleted one to reuse and use a new "id" when you do.

Then you never have to worry about truncating the file.

OR

If you have a way of truncating the file to a specified length, you can simply load the last record (if it's not the one you are deleting) and move it to the position of the one you wish to delete, or shift them all back 1 record to preserve order. Then you can truncate the file to the desired size.

In addition, you are using mode "wb+" which is wrong, as it is supposed to truncate the file (to nothing) when opening it. what you really want is mode "r+b" which opens it for reading and writing but does not truncate. It doesn't say whether it creates the file if it doesn't exist, so you may need to check and use "w+b" only if it does not already exist.

Also, in initializeRecords, you have a line char s[30] = "". I'm not sure if this is really allowed, but the most it could do is maybe initialize the first char (s[0] = '\0';). What would be better (and easier) would be to just initialize toolsPtr like this:

fseek(fPtr, 0, SEEK_SET);
for (unsigned int i = 0; i < 10; ++i) {
    memset(toolsPtr, 0, sizeof(hardware));
    toolsPtr->record = i + 1;
    fwrite(toolsPtr, sizeof(hardware), 1, fPtr);
}

The only real difference is probably that the file will be filled with zeroes instead of random junk from memory. It tends to make a difference if you start using hexdump on the file to see what happened if something goes wrong, at the very least..

edited Feb 04 '19 at 02:31

answered Feb 04 '19 at 01:51

Bob Shaffer

635
3
13

Hi Bob. The question is taken from a textbook (C How to Program, (8e) by Deitel & Deitel), and the wb+ format is what they use. I didn't know there was an alternative way of writing it. The logic behind using that particular mode stems from the fact that the program is supposed to create the file as well as manipulate it. I agree that using r mode would make more sense, as realistically the file would probably already exist. Your comments about adding a delete field are quite interesting, and make a lot of sense. But again, because the textbook has been using examples where records were – Wade Shiell Feb 04 '19 at 02:12
basically 'wiped', I didn't even think to add another field to indicate deletion. I'll have a go at incorporating your suggestions into my program. Thanks :) – Wade Shiell Feb 04 '19 at 02:13
Wow, I learned from that book many years ago. Mine was more like 2nd edition or something like that. I think I loaned out all my copies and they never came back. The manual does say that w+ means "Open for reading and writing. The file is created if it does not exist, otherwise it is truncated. exist, otherwise it is truncated..." which tells me that, unless you want to clobber it, you should open with r+ instead. The b is a no-op on my system. Idk if it does anything anywhere else. – Bob Shaffer Feb 04 '19 at 02:29
It is correct that the b can be in be in between the r/w and + it seems, but I'm pretty sure you still want to use r+, at least if the file already exists. I've updated the answer to show this. – Bob Shaffer Feb 04 '19 at 02:33
You already have "record" which is always the same as the record offset in the file. It could be used as "id" (I missed it before) but you would have skipped record numbers when displaying the full list unless you packed it so that all the deleted ones were at the end (and possibly truncate the file). Then your records would get renumbered, and this could get confusing in a real world application. If you wanted to preserve record numbers, you need to keep track of where you are, You could use record 0 to store this kind of thing, and other data as well. – Bob Shaffer Feb 04 '19 at 02:52
If you start to use record identifiers that are not exactly equal to the record offset / sizeof(hardware), you will need to either create an index, or manually read through the database to find the record (can't just fseek any more). If you made an index, it would be very easy to just get the new, unused record number from that, as you could just leave the deleted record in the index with offset -1 or something, and then you could always get the total ever added by looking at the file size of the index and dividing it by the size of an index record. – Bob Shaffer Feb 04 '19 at 03:01
There are really not a lot of situations where it is ideal to store data this way, but I'm sure that since you are working from a textbook example that it's little more than a learning exercise. Normally it is better to store in some kind of database like SQL or to use some kind of text format that can be accessed by other programs. – Bob Shaffer Feb 04 '19 at 03:08
Thank you Bob. Definitely sounds like an example of how tackling a problem using a less than optimal solution is a good way of teaching you that there are better ways of doing things. I suspect textbook questions are often hamstringed by the fact that they ask students to solve problems using less than ideal means. Your suggestions have definitely pointed me in the right direction for a working solution though. Thank you! – Wade Shiell Feb 04 '19 at 03:59
Don't get me wrong, there are definitely uses for this. I have had use for it a few times, and anyone who has to work on storage engines for databases or database servers certainly does. You learn how to work with binary files by writing programs like that, and then later on you learn how to write programs that use libraries to access better databases. Just doing binary I/O with files like that is easy for trivial programs like that, but learning how to link to and use libraries for database servers and such will make more complex things easier and easier to integrated with other software. – Bob Shaffer Feb 04 '19 at 05:55
Stuff to look forward to once the semester starts :) – Wade Shiell Feb 04 '19 at 06:02

Inventory program, possible boundary value issue?

1 Answers1