Paul, I have incorporated all the review comments. Dosbox will work on both the architecture and the original bug/CR was raised for generic architecture. So i didn't change anything in section 9.
Pls do review it once. Location: http://cr.opensolaris.org/~an230044/dosbox/ Abhijit On 03/23/09 20:39, Paul Cunningham wrote: > See below ... > > Paul > > Abhijit Nath wrote: >> >> Thanks for your review comments. I have incorporated all except >> comment 3. > >> On 03/19/09 17:08, Paul Cunningham wrote: >>> Abhijit Nath wrote: >>>> >>>> Please help review the changes to integrate DOSBox v. 0.72 (CR >>>> 6657028). I have incorporated the basic review comments. >>>> The webrev is at: http://cr.opensolaris.org/~an230044/dosbox/ >>> >>> === Start of Comments === > > .. cut .. > >>> 2. CDDL HEADER and top of files (various files) >>> Cosmetic: Change the tops of files so that they >>> conform to that in .. >>> "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/" >> > > Various files are still wrong, eg. > a) extra # line at top > b) missing line-space after CDDL HEADER END header > > >>> 3. usr/src/cmd/dosbox/Makefile.sfw >>> Change as follows .. >>> Roland Mainz wrote: >>> > use "env - ..." and not "env ..." in the Makefiles >>> > to make sure "configure"&&"make" only see the env >>> > variables they should really get (and not pick-up >>> > any random env variable). >>> > use either $(SHELL) or /usr/bin/bash for "configure" >>> > calls (so we know which one is used and "configure" >>> > doesn't pick one itself). > > As per previous reply; you should apply this > > Do you still need the line ... > 56 INSTALL="$(INSTALL) -c" > > > .. cut .. > >>> 5. usr/src/cmd/dosbox/sunman-stability >>> Is the following state correct ? .. >>> 22 Interface Stability Volatile > > Is 'Volatile' really the correct state for this ? > > > .... cut ... > >>> 9. usr/src/pkgdefs/SUNWdosbox/prototype_com >>> I assume this only works on i386, so shouldn't the >>> files be in SUNWdosbox/prototype_i386 rather than in >>> here ? > > What was your decision here ? > > Also if it is only i386, in usr/src/pkgdefs/Makefile shouldn't > 'SUNWdosbox' be in the i386_SUBDIRS= list rather than the > COMMON_SUBDIRS= list ? The same probably also applies to > usr/src/cmd/Makefile > >>> === End of Comments ===== >
