Hello, I am Dhiren. Presently I have been preparing a software design for a product developed by our team. My aim was to make the software maintainable and secure. Some of the details of the software design are as follows:
1 All the global variables are structures containing members required for different operations in the product
2 All the functions used in different modules such as reading ADC or taking action based on status etc are called in main.c inside the infinite loop. Timer isr is used for delays
3 Each module function will be called in main. c with arguments which are global structures passed as const and the return values of these functions are updated in global structure in main. c. These individual function return values are local structures of the same type as the respective global structures
The layout of the above explained program is as below:

//File1 main.c
struct1_typedef struct1;//global structure variable struct1
struct2_typedef struct2;//global structure variable struct2

void main(void)
{
  for(;;) ;
  {
    struct2 =fn(&struct1) ;//The function called  inside infinite which                                //reads struct1 as const pointer // 
                   //and struct2 is updated by return value of function fn
  }
}

//File2 task.c
struct2_typedef fn(const struct1_typedef ×struct1)
{
  struct2_typedef *struct2_local;
  if( *struct1.mem1 == 0) //const structure is read
  {
    struct2_local. mem1 = 1;
  }
  return struct2_local;//local structure of the same type as the global                            //structure updated in main.c
}

Advantages:
1 There are individual files with all related routines in one file. This would cause the routines to be easily maintainable for any further changes
2 Since global variables are passed to functions via const arguments these global variables are there fore not allowed to be changed inside the module functions keeping them protected
Disadvantages:
1 The function that are defined in individual files need to have global variables whenever they want to write into global variables but they are not easily accessible until you pass them as arguments
2 Since the arguments are const you have to copy them into local structure and then return it so that they can be actually written to the global variable in main.c
3 If you need to update 2 or more structures as the members are on different structures, then you would have to make an individual structure with required members. This can update the actual structure members in main. c since the return of a function can only be a single structure

My question is whether one can improve this software design so that it is easier to access  global variables in individual files while keeping the maintainability and security of the program as above. My team is objecting to this software design precisely for the disadvantages and I am trying to convince them with the advantages for this method

You can do this of course but:

You certainly don't need pointers. That said, most of the time you would want to use them - the implicit memory copies that take place flinging structures around by value can be a real waste of CPU cycles, not to mention memory bandwidth. – Carl Norum Mar 11 '12 at 7:15

https://stackoverflow.com/questions/9653072/return...

The obvious question to be asked is what do you gain from this, that can't be had from a classic "Superloop" or using an RTOS ? For the kind of code that I write the loss of performance would be intolerable.

It kind of looks like excess complexity looking for a place to strike, you create and return a whole structure in order to return 1 element of it. And the function gets the chance to wipe out some global stuff it doesn't need access to.

Listen to your team !

MK

Yes, There would be reduction in performance if I pass structure by value and to save the structre's other members I would have to copy the original structure into the local structure. This I admit, but I wanted to check whether my application wont mind such a reduction in performance. If application demands I would send the global structure as parameter by address which can be updated in the function itself as you have mentioned. Even after doing above, the software design for the team would not change much. The team wants global structure variables to be declared inside individual files and they want it to be accessed as per their requirement. I am actually objecting to that as I want all the global variables to be defined inside one file in this case main.c. This would also make the program maintainable as all the functions in respective files would access the global variables by address via parameters and none of the files would extern these variables. This I believe would reduce the code complexity

I agree with Michael, it isn't obvious why you return struct2 as opposed to just letting fn() fill in the return information you care about in struct1.

If you really need two separate structures, eg. one to pass information to fn() and another for fn() to return information to caller, you might consider:

fn(&struct1, &struct2);

This should be more efficient for the reasons that Michael points out, and eliminates the need for a local structure in fn().

Without knowing the structure definition, it's impossible to say how inefficient this is. In general you're going to need storage to hold calling parameters and return information, so if these structures are tailored to contain only the minimal and necessary information then it may not be that inefficient. That said, instantiating lots of structures and passing them around is generally not a good way to go.

I'm a little curious about what's returned by fn(). In task.c inside fn() you have:

struct2_typedef *struct2_local;
...
return struct2_local;

Back in main you have:

struct2_typedef struct2;
...
struct2 = fn(&struct1);

Sorry if there is a gap in my understand of how the C compiler handles this situation with structure references, but it looks like you're returning a pointer and assigning it to a structure.

Sorry you pointed out right it is 

struct2_typedef struct2_local;

I have not shown any definition of the structure as I was using them as an example. The no of members in the structure can vary based on the requirement of the function as there may be more members to check, Now as you have said as shown below I would make the changes in the program

//File2 task.c
void fn(const struct1_typedef *struct1, struct2_typedef *struct2)
{
  
  if( *struct1.mem1 == 0) //const structure is read
  {
    struct2->mem1 = 1;
  }
 
}

 and in turn I would just call the above function in infinite loop. But I believe that this implementation is far better than implementation of same thing as below which my team feels is better implementation

//File2 task.c

extern struct2_typedef struct2;

void fn(const struct1_typedef *struct1)
{

  if( *struct1.mem1 == 0) //const structure is read
  {
    struct2.mem1 = 1;
  }

}

The above example unnecessarily shares the global variable struct2 as extern with the other file File2 task2.c which could make the program more complex

Ask your team a few questions:

• struct2 is declared global, while struct1 is accessed via the pointer. Why this way and not vice versa?
• Imagine, you've got 17 data structures in your main.c. Which ones should be declared global, and which ones should be passed as a pointer? Why?

You're trying out things here which is fine.

The comments contributed by others are on point - I would listen to them.

One other thing to add:

How will you test these components?

A hallmark of a good design is that it is testable at the unit level. A unit is often defined by the translation units (a fancy name for source code files).

Having a well defined interface to a source code module, using a header file to define that interface, generally considered good practice. Using the header file alone a developer or test engineer should be able to write unit tests against that module.

If you want the easy answer to "did I write this code well?" The answer is almost always found in the answer to this question: "Did I write my unit tests?".

Where I work we spend as much time writing tests as we do writing code.

I would get rid of the const in the argument, since you are overwriting stuff in the structure anyway.  

This would make it more obvious what the intent of the function is.  

You should think about whether this global structure is the right thing to do.  Generally, all functions don't require access to all the members in the global structure.  Maybe your functions should be smaller so that thaey operate only on certain parts of the structure.  And your structure should be broken up into smaller structures.

Remember a couple of tenets of software design: cohesiveness and coupling.  Minimize coupling, maximize cohesion.  Passing a global structure around does not minimize coupling.  Breaking a function down into small pieces maximizes cohesion, and breaking shared data into smaller pieces minimizes coupling.

I appreciate your comments especially mathew barr,natersoz and jmford94. I do feel that allowing module functions directly accessing global structures via pointers is not right even though it might increase the readability in the individual module functions. Also as per natersoz it would make unit testing difficult. I feel using interface functions for each module to access their updated values by other funtions as required is a good idea and I would incorporate it in my software design. Thank you all for your comments. I hope to share my new updated software design for more comments

1. Your code example will not compile.

2. You are passing an address of a global structure. Ok, modify whatever you need via the pointer, and return nothing from the function.