On Mon, Jun 09, 2003, Adam McDaniel wrote:
> Actually, this argument seems moot in my environment where my
> SEARCH_DIR would have been /usr/local/palm/arm-palmos/lib/ .. is this
> really required?
How would I know? It was you (and Alexander) that included it in the
patch ;-)
> This endianutils.h file was actually included as an example from palm
> for building armlets. The code itself is straight forward enough and
> free for general use.
Normally, I would like to see the license; or a document that says
that it is in public domain. However, the code in endianutils.h isn't
exactly a Palm invention (except maybe for the macro names); the macros
I can find even in the source code for the Linux kernel ;-)
Anyway, please don't include code you haven't written yourself without
telling me.
> Actually, it technically does.
No, it doesn't. It depends on the rcp file; if the source code
is changed then it is the Makefile in the armlets dir that should
rebuild the armlets stuff and update the rcp file, so that the
viewer's Makefile will rebuild the viewer.
> Same reason as above.
Still wrong, though ;-)
> I didn't think it was that bad, except for a few variable names could
> be more specific.
Well, it's not. Consider the '!' to be a result of the patches I have
reviewed today and yesterday, i.e. a general request to at least try
to use the same style as the existing code (and most of the code I
reviewed yesterday you had already added to CVS:)
> Can you provid esome examples?
- numeric expressions in number-line order
- data types (e.g. it would feel safer if instead of short the code
used signed short for i,j, and top in ArmDataStruct,
char wasn't left to get whatever the default is, and
when we deal with byte swapping and endianess it's
usually a safer bet to use the same type on both sides
of the '=' sign:)
- comments about limitations that you should be aware of, e.g. that
the search armlet only handles latin1 (not multibyte)
a README file in the armlets dir would be nice
BTW, the useArmlet variable seems to be a bit of fluff; the armChunkH
(we can discuss the choice of name, though) already indicates if we
are using the armlet or not, i.e. is it NULL or not. Use it and you
can drop the MemPtrRecoverHandle, too.
> Armlets are generally a good thing! And should be included if
> we can build it.
Then enable it with the option. Considering the problems I find now
and then because someone (no names:) doesn't try to build the viewer
without hires support I will probably also change the hires option
to "make it easier" for that individual to build the non-hires
version ;-)
> Although the overhead in this is only ~500 bytes, it would be nice to
> keep the default as on unless it's impossible to build.
If you think it is difficult to run configure, then write a shell
script to do it for you ;-)
> You know, innocent until proven guilty? :)
cvs log hires.h | grep -A2 "revision 1.27"
/Mike
_______________________________________________
plucker-dev mailing list
[EMAIL PROTECTED]
http://lists.rubberchicken.org/mailman/listinfo/plucker-dev