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

Re: [XaraXtreme-dev] [patch] Have implemented a Linux port of GetMemoryStatus ("Function to find available RAM")


Alex Bligh wrote:
> Israel,
> OK I committed this. A couple of notes:
> 1. Rather than relying on __WXMAC__ being the only other wx port,
>    I explicitly tested for __WXGTK__. I take the point about assuming
>    if this is set that it's Linux or something with /proc/memory as
>    it fails gracefully. Perhaps someone with FreeBSD would like to
>    provide an equivalent routine.
That makes sense, since the code fails gracefully there's nothing lost
in trying /proc by default.

> 2. Your patch didn't apply using patch (don't know why) so I had
>    to apply manually. "svn diff" from the root directory of the
>    source produces names like "wxOil/memory.cpp" not "memory.cpp".
>    I couldn't immediately see why it didn't apply. Perhaps it wasn't
>    against a completely clean copy of the code.
Ah, I made the diff from within wxOil, that must have been the problem -
the path wasn't the same.

I noticed you missed a small change, though; I had added the following,
near the top of the file (after all the includes and before the
Technical Notes):

#if !defined(__WXMSW__) && !defined(__WXMAC__)
#  include <stdio.h>

The include is necessary since the code is using stdio functions. Of
course, to be consistent with the changes you made, it should instead be
something like:

#if !defined(__WXMSW__) && defined(__WXGTK__)
#  include <stdio.h>

> 3. Normally we avoid types "int" and "long" preferring the fixed
>    length types INT32 and (for instance) INT64. Your use of them
>    here is quite correct (as sscanf with a %lu will expect a pointer
>    to a 'long' - i.e. a different length integer on a 32 and
>    64 bit platform). However, there is a devious script called
>    normalise.pl which goes through fixing up "long" (which people
>    10 years ago seemed to use to mean "a 32 bit number" in the
>    Camelot source) and changing it to INT32. To stop that happening
>    and indicate to the reader you really did mean a "long", we put
>    in "/*TYPENOTE: Correct*/" - a Camelot eccentricity...
Oh, I see; I was not aware of that. Well, all should be well, with the
TYPENOTE comments, then.

> Thanks for the patch!
You're quite welcome, it's my pleasure to contribute!