ORBit_copy_value_core() bug?

[Jules Colding]

Hi,

I might just be suffering from coffee deficiency but shouldn't the copy
of sequences and arrays be walking the sequence/array members?


Thanks,
  jules



Index: corba-any.c
===================================================================
--- corba-any.c (revision 2040)
+++ corba-any.c (working copy)
@@ -934,13 +934,13 @@
  pval1 = ((CORBA_Principal *)*val)->_buffer;
 
  for (i = 0; i < ((CORBA_Principal *)*newval)->_length; i++)
- ORBit_copy_value_core (&pval1, &pval2, tc->subtypes [0]);
+ ORBit_copy_value_core (&pval1[i], &pval2[i], tc->subtypes [0]);
  *val = ((guchar *)*val) + sizeof (CORBA_sequence_CORBA_octet);
  *newval = ((guchar *)*newval) + sizeof (CORBA_sequence_CORBA_octet);
  break;
  case CORBA_tk_array:
  for (i = 0; i < tc->length; i++)
- ORBit_copy_value_core (val, newval, tc->subtypes[0]);
+ ORBit_copy_value_core (val[i], newval[i], tc->subtypes[0]);
  break;
  case CORBA_tk_fixed:
  g_error ("CORBA_fixed NYI!");
 

_______________________________________________
orbit-list mailing list
orbit-list@...
http://mail.gnome.org/mailman/listinfo/orbit-list

[Michael Haubenwallner]

On Wed, 2007-12-12 at 14:44 +0100, Jules Colding wrote:

> Hi,
>
> I might just be suffering from coffee deficiency but shouldn't the copy
> of sequences and arrays be walking the sequence/array members?
>
>
> Thanks,
>   jules
>
>
>
> Index: corba-any.c
> ===================================================================
> --- corba-any.c (revision 2040)
> +++ corba-any.c (working copy)
> @@ -934,13 +934,13 @@
>   pval1 = ((CORBA_Principal *)*val)->_buffer;
>  
>   for (i = 0; i < ((CORBA_Principal *)*newval)->_length; i++)
> - ORBit_copy_value_core (&pval1, &pval2, tc->subtypes [0]);
> + ORBit_copy_value_core (&pval1[i], &pval2[i], tc->subtypes [0]);

AFAICT ORBit_copy_value_core() increments both 'pval' and 'pval2' by
sizeof(CORBA_type), so subsequent call operates on next array element.
Additionally, compiler would index _bytes_ here, not _types_.

/haubi/


--
19. - 21. Februar 2008
Salomon Automation auf der LogiMAT 2008, Neue Messe Stuttgart, Deutschland
Halle 6, Stand 527

23. - 27. Februar 2008
MoveRetail auf der EuroShop 2008 in Düsseldorf, Deutschland
Halle 6, Stand C50





Salomon Automation GmbH - Friesachstrasse 15 - A-8114 Friesach bei Graz
Sitz der Gesellschaft: Friesach bei Graz
UID-NR:ATU28654300 - Firmenbuchnummer: 49324 K
Firmenbuchgericht: Landesgericht für Zivilrechtssachen Graz

_______________________________________________
orbit-list mailing list
orbit-list@...
http://mail.gnome.org/mailman/listinfo/orbit-list

[Jules Colding]

Hi,

On Wed, 2007-12-12 at 15:11 +0100, Michael Haubenwallner wrote:

> On Wed, 2007-12-12 at 14:44 +0100, Jules Colding wrote:
> > Hi,
> >
> > I might just be suffering from coffee deficiency but shouldn't the copy
> > of sequences and arrays be walking the sequence/array members?
> >
> >
> > Thanks,
> >   jules
> >
> >
> >
> > Index: corba-any.c
> > ===================================================================
> > --- corba-any.c (revision 2040)
> > +++ corba-any.c (working copy)
> > @@ -934,13 +934,13 @@
> >   pval1 = ((CORBA_Principal *)*val)->_buffer;
> >  
> >   for (i = 0; i < ((CORBA_Principal *)*newval)->_length; i++)
> > - ORBit_copy_value_core (&pval1, &pval2, tc->subtypes [0]);
> > + ORBit_copy_value_core (&pval1[i], &pval2[i], tc->subtypes [0]);
>
> AFAICT ORBit_copy_value_core() increments both 'pval' and 'pval2' by
> sizeof(CORBA_type), so subsequent call operates on next array element.
> Additionally, compiler would index _bytes_ here, not _types_.

You are probably right, but I must admit that I do not fully understand
this part of the source. I just need some time I'm sure...

Thanks,
  jules



_______________________________________________
orbit-list mailing list
orbit-list@...
http://mail.gnome.org/mailman/listinfo/orbit-list

[michael meeks]

On Thu, 2007-12-13 at 10:36 +0100, Jules Colding wrote:
> You are probably right, but I must admit that I do not fully
> understand this part of the source. I just need some time I'm sure...

        The 'any' code is full of this rather interesting iteration pattern,
and packed full of fun indirection. OTOH - it turns out that is the best
way to have the least code that is the most flexible & re-usable.

        HTH,

                Michael.
>
--
 michael.meeks@...  <><, Pseudo Engineer, itinerant idiot


_______________________________________________
orbit-list mailing list
orbit-list@...
http://mail.gnome.org/mailman/listinfo/orbit-list

[Jules Colding]

Hi Michael,

On Thu, 2007-12-13 at 10:32 +0000, Michael Meeks wrote:
> On Thu, 2007-12-13 at 10:36 +0100, Jules Colding wrote:
> > You are probably right, but I must admit that I do not fully
> > understand this part of the source. I just need some time I'm sure...
>
> The 'any' code is full of this rather interesting iteration pattern,
> and packed full of fun indirection. OTOH - it turns out that is the best
> way to have the least code that is the most flexible & re-usable.

Thought so ;-)

It would be nice if the working of it was documented in the code. If you
ever get hit by a rampaging asteroid or just goes MIA then this
"interesting" code would risk being non-maintained as most developers
would just turn away after one glance.

In a prefect world all code is obvious by first (or second glance)
without any need for comments, but in this case I'm sure a few helping
words would go a long way towards making it easier for newcomers to help
maintaining.


Best regards,
  jules


_______________________________________________
orbit-list mailing list
orbit-list@...
http://mail.gnome.org/mailman/listinfo/orbit-list

[michael meeks]

On Thu, 2007-12-13 at 11:43 +0100, Jules Colding wrote:
> > The 'any' code is full of this rather interesting iteration pattern,
> > and packed full of fun indirection. OTOH - it turns out that is the best
> > way to have the least code that is the most flexible & re-usable.
>
> It would be nice if the working of it was documented in the code.

        It has a fairly full regression test-suite; eg. making your change
would completely break make check ;-)

>  If you ever get hit by a rampaging asteroid or just goes MIA then this
> "interesting" code would risk being non-maintained as most developers
> would just turn away after one glance.

        Nah - the other Michael understood it; Elliot Lee understands it, Dick
Porter had something to do with it, and - ultimately it's not impossible
opaque (AFAIR).

> In a prefect world all code is obvious by first (or second glance)
> without any need for comments, but in this case I'm sure a few helping
> words would go a long way towards making it easier for newcomers to help
> maintaining.

        Yes - unfortunately, walking types in the C ABI reliably &
automatically is not a particularly trivial task ;-)

        Regards,

                Michael.

--
 michael.meeks@...  <><, Pseudo Engineer, itinerant idiot


_______________________________________________
orbit-list mailing list
orbit-list@...
http://mail.gnome.org/mailman/listinfo/orbit-list