0

In the past I've struggled with understanding pointer arithmetic, typecasting and array of structs, and sometimes even if the program works, it's still technically incorrect, so I just wanted to run this bit of code that I wrote by you guys and make sure I'm understanding all the concepts right right.

#include <stdio.h>
#include <stdlib.h>

typedef struct
{
  int id, jobType;
  float salary;
} Employee;

Employee * readFileInfo(FILE *fp)
{
  int numEmployees = 0;
  fscanf(fp, "%d", &numEmployees); //find the int at the start of the file to indicate how many employees there are
  int *array = malloc( (sizeof(int)) + ( sizeof(Employee) * numEmployees ) ); //allocate an extra spot for an integer
  *array = numEmployees; //hide the number of employees one integer behind the start of the array, increment on next line
  array++;
  Employee *test = (Employee *) array;
  for(int i = 0; i < 3; i++)
    fscanf(fp, "%d %d %f", &test[i].id, &test[i].jobType, &test[i].salary); //loop through and assign each value to its proper variable
  return test;
}

Employee * getEmployeeByID(Employee *arr, int ID)
{
  for(int i = 0; i < *((int*)arr - 1); i++)
    if(arr[i].id == ID)
        return ((arr) + i); //returns address of specified employee
   return 0;
}

void setID(Employee *p, int ID)
{
  p->id = ID;
  printf("\nset successfuly\n");
}

int getSize(Employee *arr)
{
  return *((int*)arr - 1); //returns the int value hidden before start of array
}

 void getAvgSalary(Employee *arr) //calculates average salary, prints out all employees who make more than that
 {
   float total = 0, avg = 0;
   for(int i = 0; i < getSize((Employee*)arr); i++)
     total += arr[i].salary;

   avg = total / (float)getSize((Employee*)arr);

   for(int i = 0; i < getSize((Employee*)arr); i++)
    if(arr[i].salary >= avg)
        printf("ID: %d has above the avg salary with a salary of $%.2f\n", arr[i].id, arr[i].salary);
 }

 int main(void)
 {
   FILE *fp = fopen("test.txt", "r+");
   Employee *array = readFileInfo(fp);
   Employee *emp2 = getEmployeeByID(array, 2);
   setID(emp2, 5);
   getAvgSalary(array);
 }

test.txt
3 1 5 20000 2 1 100000 3 3 50000

Basically, the program is supposed to open test.txt, calls readFileInfo which finds the first integer in the file (which signifies the number of employees to follow), allocates memory for an array of structs using 3 things: that size number multiplied by the size of the employee struct, with one extra integer added to "hide" that size variable before the start of the array. After that, various functions are called on the array to experiment around with typecasting / pointer arithmetic

Any help, corrections, advice, observations, or responses at all would be great!

Azoros
  • 13
  • 1
  • Dear OP, code that works but may be improved or review is much more likely to be better-suited to [C.R.S.E](https://codereview.stackexchange.com) , since that S.E. site exists *precisely* for this . – A P Jo Sep 22 '20 at 03:54
  • @APJo sorry! didn't know that was a thing, i'll post there now. thank you! – Azoros Sep 22 '20 at 03:57
  • This breaks strict aliasing. For what you want I suggest a new structure which holds the array length (using the more appropriate `size_t` type) and a [flexible array member](https://en.m.wikipedia.org/wiki/Flexible_array_member) for the `Employee` array. – Some programmer dude Sep 22 '20 at 04:03
  • Dear OP, its completely OK :) – A P Jo Sep 22 '20 at 06:55

1 Answers1

1

The code is not correct. Assume the float was instead a double, the entire struct would then have the alignment requirement that of a double, which would be 8 bytes on x86-64. But you've forced it to alignment 4. It has undefined behaviour which means that it can work, or it can fail catastrophically.

Fortunately there is zero need for this hack though. Just use a struct with a flexible array member instead:

typedef struct
{
  int id, jobType;
  float salary;
} Employee;

typedef struct {
  size_t numEmployees; // use size_t for array sizes and indices
  Employee employees[];
} EmployeeArray;

// allocate sizeof (EmployeeArray) space for the struct
// and sizeof (Employee) space for each array element
EmployeeArray *array = malloc(sizeof *array + sizeof (Employee) * numEmployees);
array->numEmployees = numEmployees;
array->employees[0] = ...