[Libvir] [PATCH] Support Sun's Compiler

[Mark Johnson]

On 9/7/07, Daniel P. Berrange <berrange at redhat.com> wrote:
> On Fri, Sep 07, 2007 at 11:33:48AM -0400, Mark Johnson wrote:
> > The following patch allows libvirt to be compiled
> > with Sun's CC compiler.
>
> Small request - for any future patches could you make sure gmail sends
> them as text/plain rather tha application/octet-stream, or have them
> inline instead of attachments - makes it easier to reply quoting the
> patch.

Will do..



> So, the bulk of this patch is just getting rid of the anonymous members
> in the union. Looks huge, but its an obvious safe fix - I explored doing
> this myself before to make us compile in c89 mode, but decided it wasn't
> worth it at the time, since we've a tonne of other stuff which breaks in
> c89 mode already.
>
> I'm rather puzzelled about this:
>
> -            free(names[i]);
> +            free((void *)names[i]);
>
> There should never be any need to cast to (void*)  as far as I understand
> things. There's a couple more examples of this. What error does the Sun
> compiler give without this explicit cast ?

Hmm, strange... Yeah a cast is not needed here.  Was this something other
than a char ** sometime in the past?  I will yank those out.


>
> For this chunk, I assume the compiler was complaining about unreachable
> code since all branches in the switch() returned ?

Yes.



>      default:
> -        return gettext_noop("no state");  /* = dom0 state */
> -    }
> -    return NULL;
> +        ;/*FALLTHROUGH*/
> +    }
> +    return gettext_noop("no state");  /* = dom0 state */
>
>
>
> Can you explain this chunk:
>
>  /* As of Hypervisor Call v2,  DomCtl v5 we are now 8-byte aligned
>     even on 32-bit archs when dealing with uint64_t */
> +#ifdef __linux__
>  #define ALIGN_64 __attribute__((aligned(8)))
> +#else
> +#define ALIGN_64
> +#endif
>
> The alignment to 8 byte boundaries is neccessary for the Xen Dom0 ABI when
> running on 32-bit platforms since it has to be 32/64-bit invariant. Is this
> a mistake, or is the Solaris 32-bit Xen ABI different ?  If so can you add
> a comment about the Solaris ABI difference so we remember the reason for this
> conditional in the future.

It's a mistake.  I'll remove this too...


I'll re-submit the patch later with the fixes...




Thanks,

MRJ