Re: [pdf-devel] Patch for FS#93 stack based iterators

[Michael Gold]

On Tue, Jan 27, 2009 at 16:06:14 -0800, address@hidden wrote:
>  > Date: Tue, 27 Jan 2009 16:20:31 -0500
>  > From: Michael Gold <address@hidden>
>  > Content-Disposition: inline
>  > 
>  > They're 56 bytes on x86_64. Since all the struct's values are pointers
>  > or size_t (which is pointer-sized on x86/x86_64), it would make sense to
>  > use a type like intptr_t instead of char.
> 
> Yes, I discussed this on IRC with jemarch, what about a 'void*' pointer ?

That should work too.

>  > We should also verify
>  >        sizeof(pdf_list_iterator_s) >=3D sizeof(gl_list_iterator_t)
>  >        and sizeof(pdf_hash_iterator_s) >=3D sizeof(gl_list_iterator_t)
>  > 
>  > Is there a portable way to do this at compile time? If not, assertions
>  > could be used in pdf_{list,hash}_iterator.
> 
> If it is, then we can just use the gl real size. :-)

> BTW, I didn't use sizeof() on gl_list because that way we need to make
> gnulib implementation public. :-/

I think you're referring to sizing gl_itr based on
sizeof(gl_list_iterator_t) -- if so, you're right that this won't work.

What I meant was verifying those conditions in our private code (not the
public header file), as a sanity check.

> OTOH, I'm really against assertions in code. I consider it a serious design
> flaw.

Why?

If the gnulib struture was changed and became bigger than ours in some
future version, an assertion would ensure it gets noticed (since the
unit tests would fail, assuming NDEBUG wasn't defined).

> So if we can't get the size at compile time, I'd suggest either to ask the
> gnulib guys to provide the size or give a large enough size, whatever that
> means. :-)

We can get the size, since sizeof() is calculated at compile time, but
making the compilation fail if our struct is too small would allow this
potential problem to be caught more quickly.

This page describes a way to implement compile-time assertions in C:
  http://www.embedded.com/columns/programmingpointers/164900888?printable=true
But I'm not suggesting we do it right now, since it doesn't seem
entirely straightforward.

-- Michael

signature.asc
Description: Digital signature