Databases
StackOverflow2013
Postsdbo
PO
 

Note that there are some explanatory texts on larger screens.

plurals
  1. PO
    primarykey
    data
    text
    <p>I see several issues in the posted code, each of which could cause problems:</p> <h3>returning the new array</h3> <p>Your function is taking an <code>int* array</code> but then you are trying to swap it with your <code>temp</code> variable at the end prior to returning the new array. This will not work, as you are simply replacing the local copy of <code>int* array</code> which will disappear after you return from the function.</p> <p>You either need to pass your array pointer in as an <code>int**</code>, which would allow you to set the actual pointer to the array in the function, or, I would suggest just returning a value of int* for your function, and returning the new array.</p> <p>Also, as mentioned in <a href="https://stackoverflow.com/questions/7097678/removing-elements-from-dynamic-arrays/7097995#7097995" title="this answer">this answer</a>, you really don't even need to reallocate when deleting an element from the array, since the original array is big enough to hold everything.</p> <h3>size and offset calculations</h3> <ol> <li><p>You are using <code>sizeof(int*)</code> for calculating the array element size. This may work for some types, but, for instance, for a <code>short</code> array <code>sizeof(short*)</code> does not work. You don't want the size of the pointer to the array, you want the size of the elements, which for your example should be <code>sizeof(int)</code> although it may not cause problems in this case. </p></li> <li><p>Your length calculation for the offsets into the arrays looks ok, but you're forgetting to multiply the number of elements by the element size for the size parameter of the memcpy. e.g. <code>memcpy(temp, array, indexToRemove * sizeof(int));</code>.</p></li> <li><p>Your second call to memcpy is using <code>temp</code> plus the offset as the source array, but it should be <code>array</code> plus the offset. </p></li> <li><p>Your second call to memcpy is using <code>sizeOfArray - indexToRemove</code> for the number of elements to copy, but you should only copy <code>SizeOfArray - indexToRemove - 1</code> elements (or <code>(sizeOfArray - indexToRemove - 1) * sizeof(int)</code> bytes</p></li> <li><p>Wherever you are calculating offsets into the temp and array arrays, you don't need to multiply by sizeof(int), since pointer arithmetic already takes into account the size of the elements. (I missed this at first, thanks to: <a href="https://stackoverflow.com/questions/7097678/removing-elements-from-dynamic-arrays/7097871#7097871">this answer</a>.)</p></li> </ol> <h3>looking at incorrect element</h3> <p>You are printing <code>test[16]</code> (the 17th element) for testing, but you are removing the 16th element, which would be <code>test[15]</code>.</p> <h3>corner cases</h3> <p>Also (thanks to <a href="https://stackoverflow.com/questions/7097678/removing-elements-from-dynamic-arrays/7097831#7097831">this answer</a>) you should handle the cases where <code>indexToRemove == 0</code> and <code>indexToRemove == (sizeOfArray - 1)</code>, where you can do the entire removal in one memcpy.</p> <p>Also, you need to worry about the case where <code>sizeOfArray == 1</code>. In that case perhaps either allocate a 0 size block of memory, or return null. In my updated code, I chose to allocate a 0-size block, just to differentiate between an array with 0 elements vs. an unallocated array. </p> <p>Returning a 0-size array also means there are no additional changes necessary to the code, because the conditions before each memcpy to handle the first two cases mentioned will prevent either memcpy from taking place.</p> <p>And just to mention, there's no error handling in the code, so there are implicit preconditions that <code>indexToRemove</code> is in bounds, that <code>array</code> is not null, and that <code>array</code> has the size passed as <code>sizeOfArray</code>.</p> <h3>example updated code</h3> <pre><code>int* remove_element(int* array, int sizeOfArray, int indexToRemove) { int* temp = malloc((sizeOfArray - 1) * sizeof(int)); // allocate an array with a size 1 less than the current one if (indexToRemove != 0) memcpy(temp, array, indexToRemove * sizeof(int)); // copy everything BEFORE the index if (indexToRemove != (sizeOfArray - 1)) memcpy(temp+indexToRemove, array+indexToRemove+1, (sizeOfArray - indexToRemove - 1) * sizeof(int)); // copy everything AFTER the index free (array); return temp; } int main() { int howMany = 20; int* test = malloc(howMany * sizeof(int*)); for (int i = 0; i &lt; howMany; ++i) (test[i]) = i; printf("%d\n", test[16]); remove_element(test, howMany, 16); --howMany; printf("%d\n", test[16]); return 0; } </code></pre> <h3>a few words on memory management/abstract data types</h3> <p>Finally, something to consider: there are possible issues both with using <code>malloc</code> to return memory to a user that is expected to be <code>free</code>d by the user, and with <code>free</code>ing memory that a user <code>malloc</code>ed. In general, it's less likely that memory management will be confusing and hard to handle if you design your code units such that memory allocation is handled within a single logical code unit.</p> <p>For instance, you might create an abstract data type module that allowed you to create an integer array using a struct that holds a pointer and a length, and then all manipulation of that data goes through functions taking the structure as a first parameter. This also allows you, except within that module, to avoid having to do calculations like <code>elemNumber * sizeof(elemType)</code>. Something like this:</p> <pre><code>struct MyIntArray { int* ArrHead; int ElementSize; // if you wanted support for resizing without reallocating you might also // have your Create function take an initialBufferSize, and: // int BufferSize; }; void MyIntArray_Create(struct MyIntArray* This, int numElems /*, int initBuffSize */); void MyIntArray_Destroy(struct MyIntArray* This); bool MyIntArray_RemoveElement(struct MyIntArray* This, int index); bool MyIntArray_InsertElement(string MyIntArray* THis, int index, int Value); </code></pre> <p>etc. </p> <p>This is a basically implementing some C++-like functionality in C, and it's IMO a very good idea, especially if you are starting from scratch and you want to create anything more than a very simple application. I know of some C developers that really don't like this idiom, but it has worked well for me.</p> <p>The nice thing about this way of implementing things is that anything in your code that was using the function to remove an element would not ever be touching the pointer directly. This would allow several different parts of your code to store a pointer to your abstract array structure, and when the pointer to the actual data of the array was reallocated after the element was removed, all variables pointing to your abstract array would be automatically updated.</p> <p>In general, memory management can be very confusing, and this is one strategy that can make it less so. Just a thought.</p>
    singulars
    1. This table or related slice is empty.
    1. PORemoving elements from dynamic arrays
      singulars
      1. PTQuestion
    1. PTAnswer
    1. This table or related slice is empty.
    1. USshelleybutterfly
    plurals
    1. This table or related slice is empty.
    1. This table or related slice is empty.
    1. PORemoving elements from dynamic arrays
      singulars
      1. PTQuestion
    1. This table or related slice is empty.
    1. VO
      singulars
      1. PO
      1. This table or related slice is empty.
      1. VTUpMod
    2. VO
      singulars
      1. PO
      1. This table or related slice is empty.
      1. VTUpMod
    3. VO
      singulars
      1. PO
      1. This table or related slice is empty.
      1. VTAcceptedByOriginator
    1. This table or related slice is empty.
 

Querying!

 
Guidance
A row detail

Detail views are divided into sections. All the information in the data section comes from columns in the selected row. The other sections display data from other, related rows.

Related data can be related in a to-one or a to-many fashion. Captions of data related in a to-many fashion link to a list view showing a filtered view of the table.

Try moving around until you find a non-empty to-many entry and click on the label to get to one. You can move back to the root by clicking on the database name in the header.

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload