Optimizing code & Header Files

[Rule]

Question 1)  Optimization
In my code I am often using functions like:

Code Select Expand


void bbh_int32_copy_int(int32 *in,int *out,int n) {
   int   i;
   for( i = 0; i < n; i++ ) out[i] = in[i];
}

and so I am wondering if this is optimal code?  Is there a more efficient way to approach these sorts of tasks?  For example, using memcpy or bcopy?

Question 2) Header Files
I notice that in most header files only function prototypes are declared, but not function bodies.  Is it ordinarily considered very bad to define the body of a function in an header file?   What is the practical (e.g. efficiency) advantage to declaring function prototypes in an header file?

For example:
If in x.c I have method1() and method2(), and in y.c I use method2(), I can facilitate this simply by compiling like so: gcc -c x.c y.c , gcc x.o y.o -o work.  Or I could leave the body of method2() defined in x.c, declare the prototype to method2() in z.h, include "z.h" in x.c and y.c (or would just in y.c be sufficient?), then compile gcc -c x.c y.c, gcc x.o y.o -o workagain.   How is this latter method supposedly more efficient?  Aside from any organizational benefit, what is the point in declaring function prototypes in an header file?

Thanks.

P.S. Does compiling with the flag "-g" (used in debugging with things like Valgrind) take away from the efficiency of your compiled binaries?

[K]

If you compiled with the -Wall flag, you would receive warnings about the implicit function declaration not matching the actual function definition.   What the actual effect is, I haven't the foggiest idea.

[MyndFyre]

Question 1)  Optimization
In my code I am often using functions like:

Code Select Expand


void bbh_int32_copy_int(int32 *in,int *out,int n) {
   int   i;
   for( i = 0; i < n; i++ ) out[i] = in[i];
}

and so I am wondering if this is optimal code?  Is there a more efficient way to approach these sorts of tasks?  For example, using memcpy or bcopy?

You could use memcpy:

Code Select Expand


void bbh_int32_copy_int(int32 *in,int *out,int n) {
   int   i;
   memcpy((void*)out, (const void*)in, n * sizeof(int));
}

The problem with this code is that it's less clear what you're doing; however, a comment could clear that up easily.
The other problem is that this doesn't provide any means of error reporting.  memcpy returns void* to *out. 

That brings me to my other comment: you should prefix int* in with const to let the compiler know (and function user know) that in won't be changing for the duration of this function.

Question 2) Header Files
I notice that in most header files only function prototypes are declared, but not function bodies.  Is it ordinarily considered very bad to define the body of a function in an header file?   What is the practical (e.g. efficiency) advantage to declaring function prototypes in an header file?

The only time you can get inline functions is within a header file.

Also, your object files will be larger and your compile times will be longer if you declare a lot of function bodies in header files, *or* if you #include .c files.  Consider this:

Copy.h

Code Select Expand


HRESULT copy(void* dest, const void* src, int bytes);

Copy.1.c

Code Select Expand


/* assume appropriate headers included */
HRESULT copy(void* dest, const void* src; int bytes);

HRESULT copy(void* dest, const void* src; int bytes)
{
  void* result;
  HRESULT status;

  result = memcpy(dest, src, bytes);
  if (result == dest)
    status = E_SUCCESS;
  else
    status = E_FAIL;

  return status;
}


Copy.2.c

Code Select Expand


/* assume other appropriate headers included */
#include "copy.h"

HRESULT copy(void* dest, const void* src; int bytes)
{
  void* result;
  HRESULT status;

  result = memcpy(dest, src, bytes);
  if (result == dest)
    status = E_SUCCESS;
  else
    status = E_FAIL;

  return status;
}

CopyUser.1.c

Code Select Expand


#include "copy.c"

int main()
{
  int nums[4];
  int dest[4];
  HRESULT result;

  result = copy(dest, nums, 4 * sizeof(int));
  if (FAILED(result))
   return 1;
 
  return 0;
}


CopyUser.2.c

Code Select Expand


#include "copy.h"

int main()
{
  int nums[4];
  int dest[4];
  HRESULT result;

  result = copy(dest, nums, 4 * sizeof(int));
  if (FAILED(result))
   return 1;
 
  return 0;
}


OK.  Now that I've gone through all of that....

Copy.1.c and CopyUser.1.c use the process of including a .c file.  This is functionally equivalent to including a .h file with the function body, since #include just inserts the file text into the code before compilation begins.

What will happen is that Copy.1.c will produce Copy.1.obj when compiled, and CopyUser.1.c will produce CopyUser.1.obj which includes all of the contents of Copy.1.obj because they were compiled too.  The result is a longer compilation time and duplication of code.

Copy.2.c will produce Copy.2.obj as well, but when CopyUser.2.c includes Copy.2.h, it only includes the declaration of the copy function.  This means that the object file is not duplicated into CopyUser.2.obj; the arranging of the two functions is done through the linker which merges the object files into the executable.

When possible, it is best to keep declarations and bodies out of header files.  This is of course excepted for inline functions, which require the body to be defined in the header file, because if the function isn't compiled into the object file (as opposed to linked), it's not going to run.

[Maddox]

Could shorten it. 

Code Select Expand


void bbh_int32_copy_int(int32 *in, int *out, int n) {
     while (n--)
          out[n] = in[n];
}


[Win32]

void bbh_int32_copy_int(int32 *in,int *out,int n) {
   int   i;
   for( i = 0; i < n; i++ ) out = in;
}

First of all, I would suggest using the __fastcall calling convention, you have very few parameters and two can easily fit into ECX:EDX, while only one will be pushed onto the stack.,

If I were to write that routine, I would do;

Code Select Expand


Void __fastcall bbh_int32_copy_int(Int* piIn, Int* piOut, Int iCount)
{
/*
* ECX - 'piIn'
* EDX - 'piOut'
* EDI - 'iCount'
* EBX - Used an an intermediate between memory copying.
*
*/
__asm
{
mov edi, [iCount]

/*
* Copy the desired amount of data from the source into the destination.
*
*/
REITERATE:
// Copy the current source element to it's corrosponding destination element.
mov ebx, [ecx]
mov [edx], ebx

// Move on to the next element and adjust the copy-count accordingly.
add ecx, 4
add edx, 4
dec edi

// IF we haven't copied the desired number of dword's yet, then retierate.
cmp edi, 0
jnz REITERATE
}
}


Although, if I were to write a routine which performs the same job it would again look much different.


-Matt

[Kp]

First of all, I would suggest using the __fastcall calling convention, you have very few parameters and two can easily fit into ECX:EDX, while only one will be pushed onto the stack.

Please stop resurrecting dead threads.  Also, note that Rule is using gcc.  The Windows port of gcc recognizes __fastcall, but other builds do not.  This is for the best, since __fastcall is inferior to gcc's own register calling convention (which can pass three arguments in registers - eax, edx, ecx).  The gcc 3.4 and gcc 4.x lines are even capable of changing the calling convention automatically, if they can prove that all callers can be fixed to handle the new convention.