Re: [PATCH] Support Sun's Compiler

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 9/7/07, Daniel P. Berrange <berrange@xxxxxxxxxx> 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

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]