Bug#553593: Acknowledgement (xulrunner: Please port to sh4)

Mike Hommey mh at glandium.org
Sat Dec 19 10:02:09 UTC 2009


Hi,

On Wed, Dec 16, 2009 at 11:26:53AM +0900, Nobuhiro Iwamatsu wrote:
<snip>
> > - Please add the license boilerplate at the top of the files you add,
> >  with proper contributor listing. (See the files in the same directory
> >  for examples)
> OK. I attached updated patch.

One more nitpick: Please properly fill the original code author,
contributors list, and copyright years. I'd expect to see your name at least
in contributors. The initial developer may also not be Netscape/Mozilla,
depending on how much of the code is novel work. I doubt this to be the
case if the code merely implements the given interfaces.

> > - ifeq (sh,$(findstring sh,$(OS_TEST))) seems a bit risky. What is the
> >  platform name ? sh only ? filter would be better than findstring,
> >  then. If it is shsomething, then filter sh% would be better.
> 
> The judgment of the platform is difficult in SH.
> For example, sh has sh1 sh2 sh3 sh4 sh5, and sh3 sh4 is bi-endian.
> When user writes it as sh3el, there is little of sh3. Or there is it
> when usr writes it as sh3.
> big is sh3eb, and littte is sh3 in many cases.
> I become much quantity only in a judgment sentence when I write all these.
> Because I was simple and wanted to assume it sh*, I wrote it like a patch.
> Of course I understand in the problem that you pointed out.
> 
> I write it below, I think that I should be able to support only sh4.
> cpu-target of sh4 is sh4 and sh4a now. (There is sh4al, too, but ABI
> is different.)
> Therefore, I can revise this part as follows.
> 
> ifneq (,$(filter sh4 sh4a,$(OS_TEST)))
>         CPPSRCS                := xptcinvoke_linux_sh.cpp xptcstubs_linux_sh.cpp
> endif
> 
> Do you think so?

This sounds good. I have another concern, though. Is the code
linux-specific or would it work on, say, NetBSD ? If the latter, then
maybe renaming the files to *_unix_* would be better. If the former, you
should add an OS_ARCH test.

Cheers,

Mike





More information about the pkg-mozilla-maintainers mailing list