Printing a range of numbers using configurable worker threads

Question

I have written code to print a range of numbers using multi-threaded programming. It takes in the number of threads to spawn as input.

It is working fine and giving the expected results. I just wanted you to check it and see if the code can be written a bit more efficiently.

 #include<stdio.h>
 #include <windows.h>
 void print(LPVOID );
 struct range
 {
 int start;
 int end;
 int threadnum;
 };
 void print(LPVOID param)
 {
 struct range *ptr=(struct range *)(param);
 for(int i=ptr->start;i<=ptr->end;i++)
 printf("\n Thread %d printed %d",ptr->threadnum,i);
 }
 int main()
 {
 int start,end,range,split;
 int threadcount=0;
 printf("\n Enter the start range :");
 scanf_s(" %d",&start);
 printf("\n Enter the End Range : ");
 scanf_s("%d",&end);
 printf("\n Enter the number of threads : ");
 scanf_s("%d",&threadcount);
 struct range **ptr;
 ptr = (struct range **)malloc(threadcount*sizeof(struct range *));
 if(ptr==NULL)
 {
 printf("\n Could not allocate memory for struct range * pointer array ");
 free(ptr);
 return 1;
 }
 DWORD *ThreadId;
 ThreadId=(DWORD *)malloc(threadcount*sizeof(DWORD));
 if(ThreadId==NULL)
 { 
 printf("\n Could not allocate memory for DWORD * pointer array ");
 free(ThreadId);
 return 1;
 }
 HANDLE *hThreadArray;
 hThreadArray=(HANDLE *)malloc(threadcount*sizeof(HANDLE));
 if(hThreadArray==NULL)
 { 
 printf("\n Could not allocate memory for hThreadArray * pointer array ");
 free(hThreadArray);
 return 1;
 }
 range=end-start+1;
 split=(range/threadcount);
 for(int i=0;i<threadcount;i++)
 {
 ptr[i]=(struct range *)malloc(sizeof(struct range));
 if(ptr[i]==NULL)
 {
 free(ptr[i]);
 }
 if(split%2==0) // if the split is even then calculated the start and end range for the worker threads as below
 {
 if(i==0)
 {
 start=start;
 end=start+split-1;
 ptr[i]->start=start;
 ptr[i]->end=end;
 ptr[i]->threadnum=i;
 hThreadArray[i] = CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)print,ptr[i],0,&ThreadId[i]);
 if(hThreadArray[i]==NULL)
 { 
 CloseHandle(hThreadArray[i]);
 }
 }
 else
 {
 start=end+1;
 end=start+split-1;
 ptr[i]->start=start;
 ptr[i]->end=end;
 ptr[i]->threadnum=i;
 hThreadArray[i] = CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)print,ptr[i],0,&ThreadId[i]);
 if(hThreadArray[i]==NULL)
 { 
 CloseHandle(hThreadArray[i]);
 }
 }
 }
 else
 {
 if(i==0)
 {
 start=start;
 end=start+split;
 ptr[i]->start=start;
 ptr[i]->end=end;
 ptr[i]->threadnum=i;
 hThreadArray[i] = CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)print,ptr[i],0,&ThreadId[i]);
 if(hThreadArray[i]==NULL)
 { 
 CloseHandle(hThreadArray[i]);
 }
 }
 else
 {
 start=end+1;
 end=start+split-1;
 ptr[i]->start=start;
 ptr[i]->end=end;
 ptr[i]->threadnum=i;
 hThreadArray[i] = CreateThread(NULL,0,(LPTHREAD_START_ROUTINE)print,ptr[i],0,&ThreadId[i]);
 if(hThreadArray[i]==NULL)
 { 
 CloseHandle(hThreadArray[i]);
 }
 }
 }
}
 WaitForMultipleObjects(threadcount, hThreadArray, TRUE, INFINITE);
 for(int i=0; i<threadcount; i++)
 {
 CloseHandle(hThreadArray[i]);
 free(ptr[i]);
 }
 }

• Aseem Bansal: Please read this about casting the result of malloc.

Answer 1

• First of all, while reviewing your code the bad code formatting style really hurts my eyes. Good code formatting and indentation makes it easier to review and maintain code.
• Get rid of all unnecessary spaces and tabs.
• Your main function is doing many things at once. Try to break the logic in smaller modules and write function for them.

• Define your structure into one type using typedef. The code below

  struct range
  {
   int start;
   int end;
   int threadnum;
  };
  

  should be like

  typedef struct range
  {
   int start;
   int end;
   int threadnum;
  }Range;
  

  So you can just use Range instead of struct range everytime.

  Then

  struct range *ptr=(struct range *)(param);
  

  will be something like

  Range *ptr = (Range *)(param);
  

• Keep one space both side of operators (=,==).

• Your main function should return EXIT_SUCCESS or return 0.

• Aseem Bansal: Good points. About casting you should also read the link in my comment on the question.