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 -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org