Plain old C code review

Started by
11 comments, last by Lode 16 years, 3 months ago
Hi, I've been converting a part of a C++ library I had made in the past, to C (C90 more specifically). However I have experience with C++, but not with C, so a lot of things came as a surprise to me (such as putting all declarations at the top of a scope, putting the struct keyword in front of everything, and no "//" comments). Because I'm inexperienced with C, but I'm thinking about releasing the C-version anyway, to avoid looking too newbie, could someone with experience in C take a look at this code? In other words, a code review :) Because the C++ code had a lot of std::vectors I made some sort of replacement in C for those std::vectors. Is this foolish or not, or what do real C coders do to get dynamic arrays with a convenient syntax? That vector stuff is only a small part of the C code, but to save you from having to look through a lot of C code, I'll only post the vector code here and one other function that uses it to emulate an "std::vector<std::vector<unsigned> >". Anyway here it is: The vectors, vector is generic, uvector is for unsigned ints and ucvector is for unsigned chars:
struct vector
{
  void* data;
  size_t size; /*in groups of bytes depending on type*/
  size_t allocsize; /*in bytes*/
  unsigned typesize; /*sizeof the type you store in data*/
};

void vector_clear(struct vector* p) /*unlike C++'s std::vector, here it just clears the memory*/
{
  p->size = p->allocsize = 0;
  free(p->data);
  p->data = NULL;
}

/*clear and use destructor on elements*/
void vector_cleard(struct vector* p, void dtor(void*))
{
  size_t i;
  for(i = 0; i < p->size; i++)
  dtor(&((char*)(p->data)));
  vector_clear(p);
}

<span class="cpp-keyword">void</span> vector_dtor(<span class="cpp-keyword">struct</span> vector* p)
{
  vector_clear(p);
}

<span class="cpp-keyword">void</span> vector_dtord(<span class="cpp-keyword">struct</span> vector* p, <span class="cpp-keyword">void</span> dtor(<span class="cpp-keyword">void</span>*))
{
  vector_cleard(p, dtor);
}

<span class="cpp-keyword">void</span> vector_resize(<span class="cpp-keyword">struct</span> vector* p, <span class="cpp-keyword">unsigned</span> size)
{
  <span class="cpp-keyword">if</span>(size * p-&gt;typesize &gt; p-&gt;allocsize)
  {
    p-&gt;allocsize = size * p-&gt;typesize * <span class="cpp-number">2</span>;
    p-&gt;data = realloc(p-&gt;data, p-&gt;allocsize);
  }
  p-&gt;size = size;
}

<span class="cpp-comment">/*resize and use destructor on elements*/</span>
<span class="cpp-keyword">void</span> vector_resized(<span class="cpp-keyword">struct</span> vector* p, size_t size, <span class="cpp-keyword">void</span> dtor(<span class="cpp-keyword">void</span>*))
{
  size_t i;
  <span class="cpp-keyword">if</span>(size &lt; p-&gt;size)
  <span class="cpp-keyword">for</span>(i = size; i &lt; p-&gt;size; i++)
  dtor(&amp;((<span class="cpp-keyword">char</span>*)(p-&gt;data)));
  
  vector_resize(p, size);
}

<span class="cpp-keyword">void</span> vector_ctor(<span class="cpp-keyword">struct</span> vector* p, <span class="cpp-keyword">unsigned</span> typesize)
{
  p-&gt;data = NULL;
  p-&gt;size = p-&gt;allocsize = <span class="cpp-number">0</span>;
  p-&gt;typesize = typesize;
}

<span class="cpp-keyword">void</span> vector_ctorl(<span class="cpp-keyword">struct</span> vector* p, size_t length, <span class="cpp-keyword">unsigned</span> typesize)
{
  vector_ctor(p, typesize);
  vector_resize(p, length);
}

<span class="cpp-keyword">void</span> vector_push_back(<span class="cpp-keyword">struct</span> vector* p, <span class="cpp-keyword">const</span> <span class="cpp-keyword">void</span>* c)
{
  size_t i;
  vector_resize(p, p-&gt;size + <span class="cpp-number">1</span>);
  <span class="cpp-keyword">for</span>(i = <span class="cpp-number">0</span>; i &lt; p-&gt;typesize; i++) ((<span class="cpp-keyword">char</span>*)p-&gt;data)[p-&gt;size * p-&gt;typesize - p-&gt;typesize + i] = ((<span class="cpp-keyword">char</span>*)c)<span style="font-weight:bold;">;
}

<span class="cpp-keyword">void</span> vector_pop_back(<span class="cpp-keyword">struct</span> vector* p)
{
  vector_resize(p, p-&gt;size - <span class="cpp-number">1</span>);
}

<span class="cpp-keyword">void</span> vector_copy(<span class="cpp-keyword">struct</span> vector* p, <span class="cpp-keyword">const</span> <span class="cpp-keyword">struct</span> vector* q) <span class="cpp-comment">/*copy q to p*/</span>
{
  size_t i;
  vector_resize(p, q-&gt;size);
  <span class="cpp-keyword">for</span>(i = <span class="cpp-number">0</span>; i &lt; q-&gt;size * p-&gt;typesize; i++) ((<span class="cpp-keyword">char</span>*)p-&gt;data)<span style="font-weight:bold;"> = ((<span class="cpp-keyword">char</span>*)q-&gt;data)<span style="font-weight:bold;">;
}

<span class="cpp-keyword">void</span> vector_swap(<span class="cpp-keyword">struct</span> vector* p, <span class="cpp-keyword">struct</span> vector* q) <span class="cpp-comment">/*they're supposed to have the same typesize*/</span>
{
  size_t tmp;
  <span class="cpp-keyword">void</span>* tmpp;
  tmp = p-&gt;size; p-&gt;size = q-&gt;size; q-&gt;size = tmp;
  tmp = p-&gt;allocsize; p-&gt;allocsize = q-&gt;allocsize; q-&gt;allocsize = tmp;
  tmpp = p-&gt;data; p-&gt;data = q-&gt;data; q-&gt;data = tmpp;
}

<span class="cpp-keyword">void</span> vector_set(<span class="cpp-keyword">struct</span> vector* p, size_t index, <span class="cpp-keyword">void</span>* value)
{
  size_t i;
  <span class="cpp-keyword">for</span>(i = <span class="cpp-number">0</span>; i &lt; p-&gt;typesize; i++) ((<span class="cpp-keyword">char</span>*)p-&gt;data)[index * p-&gt;typesize + i] = ((<span class="cpp-keyword">char</span>*)value)<span style="font-weight:bold;">;
}

<span class="cpp-keyword">const</span> <span class="cpp-keyword">void</span>* vector_get(<span class="cpp-keyword">const</span> <span class="cpp-keyword">struct</span> vector* p, size_t index)
{
  <span class="cpp-keyword">return</span> &amp;((<span class="cpp-keyword">const</span> <span class="cpp-keyword">char</span>*)p-&gt;data)[index * p-&gt;typesize];
}

<span class="cpp-keyword">void</span>* vector_back(<span class="cpp-keyword">struct</span> vector* p)
{
  <span class="cpp-keyword">return</span> (<span class="cpp-keyword">void</span>*)(&amp;((<span class="cpp-keyword">char</span>*)p-&gt;data)[p-&gt;size * p-&gt;typesize - p-&gt;typesize]);
}

<span class="cpp-comment">/*/////////////////////////////////////////////////////////////////////////////*/</span></span>

<span class="cpp-keyword">struct</span> uvector
{
  <span class="cpp-keyword">unsigned</span>* data;
  size_t size; <span class="cpp-comment">/*size in number of unsigned longs*/</span>
  size_t allocsize; <span class="cpp-comment">/*size in bytes*/</span>
};

<span class="cpp-keyword">void</span> uvector_clear(<span class="cpp-keyword">struct</span> uvector* p) <span class="cpp-comment">/*unlike C++'s std::vector, here it just clears the memory*/</span>
{
  p-&gt;size = p-&gt;allocsize = <span class="cpp-number">0</span>;
  free(p-&gt;data);
  p-&gt;data = NULL;
}

<span class="cpp-keyword">void</span> uvector_dtor(<span class="cpp-keyword">struct</span> uvector* p)
{
  uvector_clear(p);
}

<span class="cpp-keyword">void</span> uvector_resize(<span class="cpp-keyword">struct</span> uvector* p, size_t size)
{
  <span class="cpp-keyword">if</span>(size * <span class="cpp-keyword">sizeof</span>(<span class="cpp-keyword">unsigned</span>) &gt; p-&gt;allocsize)
  {
    p-&gt;allocsize = size * <span class="cpp-keyword">sizeof</span>(<span class="cpp-keyword">unsigned</span>) * <span class="cpp-number">2</span>;
    p-&gt;data = (<span class="cpp-keyword">unsigned</span>*)realloc(p-&gt;data, p-&gt;allocsize);
  }
  p-&gt;size = size;
}

<span class="cpp-keyword">void</span> uvector_resizev(<span class="cpp-keyword">struct</span> uvector* p, size_t size, <span class="cpp-keyword">unsigned</span> value)
{
  size_t oldsize = p-&gt;size, i;
  uvector_resize(p, size);
  <span class="cpp-keyword">for</span>(i = oldsize; i &lt; size; i++) p-&gt;data<span style="font-weight:bold;"> = value;
}

<span class="cpp-keyword">void</span> uvector_ctor(<span class="cpp-keyword">struct</span> uvector* p)
{
  p-&gt;data = NULL;
  p-&gt;size = p-&gt;allocsize = <span class="cpp-number">0</span>;
}

<span class="cpp-keyword">void</span> uvector_ctorl(<span class="cpp-keyword">struct</span> uvector* p, size_t length)
{
  uvector_ctor(p);
  uvector_resize(p, length);
}

<span class="cpp-keyword">void</span> uvector_ctorlv(<span class="cpp-keyword">struct</span> uvector* p, size_t length, <span class="cpp-keyword">unsigned</span> value)
{
  uvector_ctor(p);
  uvector_resizev(p, length, value);
}

<span class="cpp-keyword">void</span> uvector_push_back(<span class="cpp-keyword">struct</span> uvector* p, <span class="cpp-keyword">unsigned</span> c)
{
  uvector_resize(p, p-&gt;size + <span class="cpp-number">1</span>);
  p-&gt;data[p-&gt;size - <span class="cpp-number">1</span>] = c;
}

<span class="cpp-keyword">void</span> uvector_pop_back(<span class="cpp-keyword">struct</span> uvector* p)
{
  uvector_resize(p, p-&gt;size - <span class="cpp-number">1</span>);
}

<span class="cpp-keyword">void</span> uvector_copy(<span class="cpp-keyword">struct</span> uvector* p, <span class="cpp-keyword">const</span> <span class="cpp-keyword">struct</span> uvector* q) <span class="cpp-comment">/*copy q to p*/</span>
{
  size_t i;
  uvector_resize(p, q-&gt;size);
  <span class="cpp-keyword">for</span>(i = <span class="cpp-number">0</span>; i &lt; q-&gt;size; i++) p-&gt;data<span style="font-weight:bold;"> = q-&gt;data<span style="font-weight:bold;">;
}

<span class="cpp-keyword">void</span> uvector_swap(<span class="cpp-keyword">struct</span> uvector* p, <span class="cpp-keyword">struct</span> uvector* q)
{
  size_t tmp;
  <span class="cpp-keyword">unsigned</span>* tmpp;
  tmp = p-&gt;size; p-&gt;size = q-&gt;size; q-&gt;size = tmp;
  tmp = p-&gt;allocsize; p-&gt;allocsize = q-&gt;allocsize; q-&gt;allocsize = tmp;
  tmpp = p-&gt;data; p-&gt;data = q-&gt;data; q-&gt;data = tmpp;
}

<span class="cpp-keyword">unsigned</span>* uvector_back(<span class="cpp-keyword">struct</span> uvector* p)
{
  <span class="cpp-keyword">return</span> &amp;p-&gt;data[p-&gt;size - <span class="cpp-number">1</span>];
}

<span class="cpp-comment">/*/////////////////////////////////////////////////////////////////////////////*/</span></span>

<span class="cpp-keyword">struct</span> ucvector
{
  <span class="cpp-keyword">unsigned</span> <span class="cpp-keyword">char</span>* data;
  size_t size;
  size_t allocsize;
};

<span class="cpp-keyword">void</span> ucvector_clear(<span class="cpp-keyword">struct</span> ucvector* p) <span class="cpp-comment">/*unlike C++'s std::vector, here it just clears the memory*/</span>
{
  p-&gt;size = p-&gt;allocsize = <span class="cpp-number">0</span>;
  free(p-&gt;data);
  p-&gt;data = NULL;
}

<span class="cpp-keyword">void</span> ucvector_dtor(<span class="cpp-keyword">struct</span> ucvector* p)
{
  ucvector_clear(p);
}

<span class="cpp-keyword">void</span> ucvector_resize(<span class="cpp-keyword">struct</span> ucvector* p, size_t size)
{
  <span class="cpp-keyword">if</span>(size &gt; p-&gt;allocsize)
  {
    p-&gt;allocsize = size * <span class="cpp-number">2</span>;
    p-&gt;data = (<span class="cpp-keyword">unsigned</span> <span class="cpp-keyword">char</span>*)realloc(p-&gt;data, p-&gt;allocsize);
  }
  p-&gt;size = size;
}

<span class="cpp-keyword">void</span> ucvector_resizev(<span class="cpp-keyword">struct</span> ucvector* p, size_t size, <span class="cpp-keyword">unsigned</span> <span class="cpp-keyword">char</span> value)
{
  size_t oldsize = p-&gt;size, i;
  ucvector_resize(p, size);
  <span class="cpp-keyword">for</span>(i = oldsize; i &lt; size; i++) p-&gt;data<span style="font-weight:bold;"> = value;
}

<span class="cpp-keyword">void</span> ucvector_ctor(<span class="cpp-keyword">struct</span> ucvector* p)
{
  p-&gt;data = NULL;
  p-&gt;size = p-&gt;allocsize = <span class="cpp-number">0</span>;
}

<span class="cpp-keyword">void</span> ucvector_ctorl(<span class="cpp-keyword">struct</span> ucvector* p, size_t length)
{
  ucvector_ctor(p);
  ucvector_resize(p, length);
}

<span class="cpp-keyword">void</span> ucvector_ctorlv(<span class="cpp-keyword">struct</span> ucvector* p, size_t length, <span class="cpp-keyword">unsigned</span> <span class="cpp-keyword">char</span> value)
{
  ucvector_ctor(p);
  ucvector_resizev(p, length, value);
}

<span class="cpp-keyword">void</span> ucvector_push_back(<span class="cpp-keyword">struct</span> ucvector* p, <span class="cpp-keyword">unsigned</span> <span class="cpp-keyword">char</span> c)
{
  ucvector_resize(p, p-&gt;size + <span class="cpp-number">1</span>);
  p-&gt;data[p-&gt;size - <span class="cpp-number">1</span>] = c;
}

<span class="cpp-keyword">void</span> ucvector_pop_back(<span class="cpp-keyword">struct</span> ucvector* p)
{
  ucvector_resize(p, p-&gt;size - <span class="cpp-number">1</span>);
}

<span class="cpp-keyword">void</span> ucvector_copy(<span class="cpp-keyword">struct</span> ucvector* p, <span class="cpp-keyword">const</span> <span class="cpp-keyword">struct</span> ucvector* q) <span class="cpp-comment">/*copy q to p*/</span>
{
  size_t i;
  ucvector_resize(p, q-&gt;size);
  <span class="cpp-keyword">for</span>(i = <span class="cpp-number">0</span>; i &lt; q-&gt;size; i++) p-&gt;data<span style="font-weight:bold;"> = q-&gt;data<span style="font-weight:bold;">;
}

<span class="cpp-keyword">void</span> ucvector_swap(<span class="cpp-keyword">struct</span> ucvector* p, <span class="cpp-keyword">struct</span> ucvector* q)
{
  size_t tmp;
  <span class="cpp-keyword">unsigned</span> <span class="cpp-keyword">char</span>* tmpp;
  tmp = p-&gt;size; p-&gt;size = q-&gt;size; q-&gt;size = tmp;
  tmp = p-&gt;allocsize; p-&gt;allocsize = q-&gt;allocsize; q-&gt;allocsize = tmp;
  tmpp = p-&gt;data; p-&gt;data = q-&gt;data; q-&gt;data = tmpp;
}

<span class="cpp-keyword">unsigned</span> <span class="cpp-keyword">char</span>* ucvector_back(<span class="cpp-keyword">struct</span> ucvector* p)
{
  <span class="cpp-keyword">return</span> &amp;p-&gt;data[p-&gt;size - <span class="cpp-number">1</span>];
}
</pre></div><!–ENDSCRIPT–>

A function that uses the vectors:

<!–STARTSCRIPT–><!–source lang="cpp"–><div class="source"><pre><span class="cpp-comment">/*LZ77-encode the data using a hash table technique to let it encode faster.*/</span>
<span class="cpp-keyword">void</span> encodeLZ77(<span class="cpp-keyword">struct</span> uvector* out, <span class="cpp-keyword">const</span> <span class="cpp-keyword">unsigned</span> <span class="cpp-keyword">char</span>* in, size_t size, <span class="cpp-keyword">unsigned</span> windowSize)
{
  <span class="cpp-comment">/**generate hash table**/</span>
  <span class="cpp-comment">/*table represents what would be an std::vector&lt;std::vector&lt;unsigned&gt; &gt; in C++*/</span>
  <span class="cpp-keyword">struct</span> vector table; <span class="cpp-comment">/*HASH_NUM_VALUES uvectors*/</span>
  <span class="cpp-keyword">struct</span> uvector tablepos1;
  <span class="cpp-keyword">struct</span> uvector tablepos2;
  
  <span class="cpp-keyword">unsigned</span> pos, i;
  
  vector_ctorl(&amp;table, HASH_NUM_VALUES, <span class="cpp-keyword">sizeof</span>(<span class="cpp-keyword">struct</span> uvector));
  <span class="cpp-keyword">for</span>(i = <span class="cpp-number">0</span>; i &lt; HASH_NUM_VALUES; i++)
  {
    <span class="cpp-keyword">struct</span> uvector* v = (<span class="cpp-keyword">struct</span> uvector*)vector_get(&amp;table, i);
    uvector_ctor(v);
  }

  <span class="cpp-comment">/*remember start and end positions in the tables to searching in*/</span>
  uvector_ctorlv(&amp;tablepos1, HASH_NUM_VALUES, <span class="cpp-number">0</span>);
  uvector_ctorlv(&amp;tablepos2, HASH_NUM_VALUES, <span class="cpp-number">0</span>);
  
  <span class="cpp-keyword">for</span>(pos = <span class="cpp-number">0</span>; pos &lt; size; pos++)
  {
    <span class="cpp-keyword">unsigned</span> length = <span class="cpp-number">0</span>, offset = <span class="cpp-number">0</span>; <span class="cpp-comment">/*the length and offset found for the current position*/</span>
    <span class="cpp-keyword">unsigned</span> max_offset = pos &lt; windowSize ? pos : windowSize; <span class="cpp-comment">/*how far back to test*/</span>
    
    <span class="cpp-keyword">unsigned</span> tablepos;
  
    <span class="cpp-comment">/*/search for the longest string*/</span>
    <span class="cpp-comment">/*first find out where in the table to start (the first value that is in the range from "pos - max_offset" to "pos")*/</span>
    <span class="cpp-keyword">unsigned</span> hash = getHash(in, size, pos);
    uvector_push_back((<span class="cpp-keyword">struct</span> uvector*)vector_get(&amp;table, hash), pos);
    
    <span class="cpp-keyword">while</span>(((<span class="cpp-keyword">struct</span> uvector*)vector_get(&amp;table, hash))-&gt;data[tablepos1.data[hash]] &lt; pos - max_offset) tablepos1.data[hash]++; <span class="cpp-comment">/*it now points to the first value in the table for which the index is larger than or equal to pos - max_offset*/</span>
    <span class="cpp-keyword">while</span>(((<span class="cpp-keyword">struct</span> uvector*)vector_get(&amp;table, hash))-&gt;data[tablepos2.data[hash]] &lt; pos) tablepos2.data[hash]++; <span class="cpp-comment">/*it now points to the first value in the table for which the index is larger than or equal to pos*/</span>

    <span class="cpp-keyword">for</span>(tablepos = tablepos2.data[hash] - <span class="cpp-number">1</span>; tablepos &gt;= tablepos1.data[hash] &amp;&amp; tablepos &lt; tablepos2.data[hash]; tablepos–)
    {
      <span class="cpp-keyword">unsigned</span> backpos = ((<span class="cpp-keyword">struct</span> uvector*)vector_get(&amp;table, hash))-&gt;data[tablepos];
      <span class="cpp-keyword">unsigned</span> current_offset = pos - backpos;

      <span class="cpp-comment">/*test the next characters*/</span>
      <span class="cpp-keyword">unsigned</span> current_length = <span class="cpp-number">0</span>;
      <span class="cpp-keyword">unsigned</span> backtest = backpos;
      <span class="cpp-keyword">unsigned</span> foretest = pos;
      <span class="cpp-keyword">while</span>(foretest &lt; size &amp;&amp; in[backtest] == in[foretest] &amp;&amp; current_length &lt; MAX_SUPPORTED_DEFLATE_LENGTH) <span class="cpp-comment">/*maximum supporte length by deflate is max length*/</span>
      {
        <span class="cpp-keyword">if</span>(backpos &gt;= pos) backpos -= current_offset; <span class="cpp-comment">/*continue as if we work on the decoded bytes after pos by jumping back before pos*/</span>
        current_length++;
        backtest++;
        foretest++;
      }
      <span class="cpp-keyword">if</span>(current_length &gt; length)
      {
        length = current_length; <span class="cpp-comment">/*the longest length*/</span>
        offset = current_offset; <span class="cpp-comment">/*the offset that is related to this longest length*/</span>
        <span class="cpp-keyword">if</span>(current_length == MAX_SUPPORTED_DEFLATE_LENGTH) <span class="cpp-keyword">break</span>; <span class="cpp-comment">/*you can jump out of this for loop once a length of max length is found (gives significant speed gain)*/</span>
      }
    }
    
    <span class="cpp-comment">/**encode it as length/distance pair or literal value**/</span>
    <span class="cpp-keyword">if</span>(length &lt; <span class="cpp-number">3</span>) <span class="cpp-comment">/*only lengths of 3 or higher are supported as length/distance pair*/</span>
    {
      uvector_push_back(out, in[pos]);
    }
    <span class="cpp-keyword">else</span>
    {
      <span class="cpp-keyword">unsigned</span> j;
      addLengthDistance(out, length, offset);
      <span class="cpp-comment">/*pos += (length - 1);*/</span>
      <span class="cpp-keyword">for</span>(j = <span class="cpp-number">0</span>; j &lt; length - <span class="cpp-number">1</span>; j++)
      {
        pos++;
        uvector_push_back((<span class="cpp-keyword">struct</span> uvector*)vector_get(&amp;table, getHash(in, size, pos)), pos);
      }
    }
  } <span class="cpp-comment">/*end of the loop through each character of input*/</span>
  
  <span class="cpp-comment">/*cleanup*/</span>
  <span class="cpp-keyword">for</span>(i = <span class="cpp-number">0</span>; i &lt; table.size; i++)
  {
    <span class="cpp-keyword">struct</span> uvector* v = (<span class="cpp-keyword">struct</span> uvector*)vector_get(&amp;table, i);
    uvector_dtor(v);
  }
  vector_dtor(&amp;table);
  uvector_dtor(&amp;tablepos1);
  uvector_dtor(&amp;tablepos2);
}
</pre></div><!–ENDSCRIPT–>

Thanks! Hopefully such a post is allowed &#111;n GDNet :) I use it in games at least.
Advertisement
I guess I don't understand why you wouldn't use a more modern version of C and get rid of those pesky things you mentioned at the top of your post.
Quote:Original post by longjumper
I guess I don't understand why you wouldn't use a more modern version of C and get rid of those pesky things you mentioned at the top of your post.


This is C90 code. According to both Wikipedia and the #C channel on freenode, a more modern C (= C99) is not widely supported. The gcc compiler itself has a list of important C99 things it doesn't support yet. I don't know why, but the C90 standard appears to be much more portable than the C99 one. And I wouldn't go through this conversion to C if it wasn't for portability. When using C99 I can as well stay with C++.
I am still wondering why didn't you just wrote a bunch of wrapper functions to the C++ code, create a header, extern "C" the whole file if __cplusplus is defined,
then write copies of the OO functions where instead of:

returntype object.method(args,...)


you have

returntype function(void* object,args,...)


then implement those functions on a cpp file as calls to the objects, for example:

#include "cobject.h"void* newobject(){return new cobject;}void deleteobject(void* object){delete (cobject*) object;}returntype function(void* object,args,...){ return ((cobject*)object)->function(args,...);}


No need to lose STL or anything C++ brings.

Hope you get the idea [smile].
You can very much see that it was written by a C++ programmer![smile]

I don't think C programmers would even use a dynamically sized array for this. You can just allocate a large array that will be at least as large as the largest possible size the output will compress to, and use that. Since your C-vector grows by a factor of two anyway, your current code will probably use at least as much memory on a file that doesn't compress well. I'd suggest figuring out the worst case and doing one malloc at the start.
If you want to save memory post-compression, then you can allocate an array of the exact size afterwards and copy the output into that.

Another idea is to only allocate one byte more than the input size, prepend a zero byte to the output, and constantly check that you don't exceed the output buffer size during compression. The moment you do exceed that size, you write a one in the first byte and then copy the input bytes directly to the rest of the output, wiping over the parital compression data.
The decompressor then checks the first byte to know whether decompression is necessary at all, or if the second byte onwards already contains the data.
"In order to understand recursion, you must first understand recursion."
My website dedicated to sorting algorithms
Hi

I started with C years ago and then switched to C++, and if I wanted to do something like this, I would do it very much like you did. The only thing I can say is that I would use typedef for the struct, so I didn't have to write as much:)

For examlpe:

typedef struct {  /* ... */} vector;void vector_clear(vector* p){  /* ... */}
Quote:Original post by Lode

*** Source Snippet Removed ***

A function that uses the vectors:

*** Source Snippet Removed ***



Looks nice.
If all that "struct" in the typename is not desired you could typedef your vector.

If all of your three implementations are the same you might want to use Macros to make it a bit more generic and weird, and ofcourse impossible to debug.

#include <stdio.h>#define vectorop(type, parameter1) {     type i = parameter1;     return i-1; }unsigned int ucvectorop(unsigned int param){    vectorop(unsigned char, param)}int main(int argc, char** argv) {    unsigned char a = ucvectorop(0);    printf( "%u", a );    return 0;}


You would partially loose typesafety, but you would be able to unify the implementation.

hope that helps
Quote:Original post by hydroo
If all that "struct" in the typename is not desired you could typedef your vector.


Quote:Original post by morgue
The only thing I can say is that I would use typedef for the struct, so I didn't have to write as much:)


Oh, that's very nice to know! Thanks guys!
One issue is that you have hooks for destructors, but not copying (unless I've missed something). This breaks the rule of three, however applicable or otherwise it is in the C world. vector_set() doesn't call a destructor, for example, before over-writing an element. Similar things are true of some of the other functions, too.

FYI: I noticed that vector_resize also has a memory leak when realloc fails.

Though I understand your motivation, I think the implementation is inconsistent with C++ thinking due in part to implementation problems, but also to the fact that this just isn't C++ anymore. I would find this interface strange to use in C, where types don't have behaviours. A lot of C programmers actually like this. Imposing a C++ way of thinking on C users might not sit too well with some.

I'm curious how the C community would react to an interface that looks something like the following, however:

#define array_create(type, elems) array_create_impl(sizeof(type), (elems))void *array_create_impl(size_t type_sz, size_t elems);void *array_destroy(void *array);void *array_resize(void *array, size_t elems);size_t array_length(const void *array);


The idea is that there is no struct holding all the pieces. Instead the details are packed in to the same dynamically allocated block as the array's memory.

So, array_create_impl() might be implemented something like this. I'm not being particularly careful about alignment here, and you may not agree with lowercase macro names, but you'll get the idea:

void *array_create_impl(size_t type_sz, size_t elems){    const size_t header_size = 2 * sizeof(size_t);    size_t *base = malloc(header_size + type_sz * elems);    if (base == NULL) return NULL;    base[0] = type_sz;    base[1] = elems;    return base + 2;}


This allows one to access the array naturally and drastically reduces the amount of casting needed. Certainly I wouldn't expect any more frills than those provided by the 4 functions I've declared, except perhaps for allowing custom allocation and deallocation routines.
I kind of like that idea, the_edd, maybe I'll really implement it like that. Only disadvantage is that too free it you must remember to free the bytes before it too so you have to use the array_destroy function, even if your void* array would be passed along to some external place where they've long forgotten that the bytes before it also have stuff in them.

So maybe I'll still keep the structs instead (that also saves all alignment problems), but give all the functions somewhat different names that make it look less C++ like and make it not suggest the rule of three :)

About the memory leak in resize: after looking in man realloc (which says it returns NULL if it failed), should this fix it?

  void vector_resize(vector* p, unsigned size)  {    if(size * p->typesize > p->allocsize)    {      size_t newsize = size * p->typesize * 2;      void* data = realloc(p->data, newsize);      if(data)      {        p->allocsize = newsize;        p->data = data;        p->size = size;      }    }    else p->size = size;  }


EDIT: Ok so I was picking other names than ctor and dtor, I thought about init and ....? So I typed in google: "opposite of init", and one of the first results was a gamedev.net thread about words for the opposite of initialize. Isn't GDNet great :)

[Edited by - Lode on December 30, 2007 6:09:46 PM]

This topic is closed to new replies.

Advertisement