0
\$\begingroup\$

Is this a correct way to copy elements from array origin to array location?

  #include <stdio.h>
   void copy(const int *origin, int *location, int n){
   int i;
   for(i=0;i<n;i++){
    location[i]=origin[i];
    }
  }
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Short answer, no. But answering a question like this is not code review. Can you show how that function is used in a real program? \$\endgroup\$
    – Reinderien
    Feb 16 at 4:30
  • \$\begingroup\$ This is more of a question for StackOverflow, Code Review is for working programs. \$\endgroup\$
    – Turksarama
    Feb 16 at 4:30
  • 2
    \$\begingroup\$ Why is stdio included? Anyway have a look at memcpy function... \$\endgroup\$
    – slepic
    Feb 16 at 5:23
  • \$\begingroup\$ The correct way is to not write this function unless you have specialized requirements. Just do memcpy(dst, src, n). \$\endgroup\$
    – Lundin
    Feb 17 at 7:34
2
\$\begingroup\$
  #include <stdio.h>

This function doesn't use anything from <stdio.h>, so don't waste the compiler's time by including it.

   void copy(const int *origin, int *location, int n){

I would put the destination argument first, to be consistent with Standard Library functions such as memcpy. And change n to be a size_t, so it will work with any array.

   int i;
   for(i=0;i<n;i++){

We can reduce the scope of i, by declaring it in the control expression: for (int i = 0; i < n; ++i).

     location[i]=origin[i];

We ought to tell the compiler (with restrict) that the arrays don't overlap.


I think it's simpler just to forward to memcpy():

#include <string.h>
void copy(int *restrict dest, int const *restrict origin, size_t count)
{
    memcpy(dest, origin, sizeof *dest * count);
}

But that's so simple that I don't think we want a separate function for it (particularly with such a broad name).

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Could be more memcpy() like and return dest; \$\endgroup\$ Feb 16 at 18:31
  • \$\begingroup\$ I am not a fan of removing common <xxx.h> from .c files. Too much maintenance - so I'd rather waste compiler's time than mine. Some development environments do automate this though. \$\endgroup\$ Feb 16 at 18:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.