Re: [PATCH] Drivemap module

[Pavel Roskin <proski@gnu.org>]

On Fri, 2008-06-13 at 00:43 +0200, Javier Martín wrote:

> El jue, 12-06-2008 a las 16:31 -0400, Pavel Roskin escribió:
> > Please don't add any trailing whitespace.  Line in the patch that start
> > with a plus should not end with a space or a tab.
> I could not find the line you were referring to. I could only find one
> line starting with a plus (in install_int13_handler), but it does not
> end with a space/tab. However, I did find one trailing tab after a
> comment, and removed it.

I mean the lines your patch adds.

> > Please avoid camelCase names, such as bpaMemInKb and retVal.  Local
> > variables should generally be short, like "ret" and "bpa_mem".
> > 
> > Some strings are excessively long.  While there may be exception of the
> > 80 column limit, I see a 118 character long line that can be trivially
> > wrapped.
> Both corrected.

retVal is still there.  Please call it "ret", it's the traditional name
of the variable to be returned.

>  Sorry, I work on a wide screen and, tough I tried to
> uphold the 80-char line rule, I'm not used to it. I think there was a
> switch in gedit to graphically display the 80-char limit, but I can't
> find it now...

Edit->Preferences->View->Right Margin

> OOC: do you know if there is a syntax highlighting mode for gas/x86
> assembly code in gedit? Right now the program thinks the code is C, and
> the only opcode it highlights is "int". Even a gas-only highlighting
> (i.e. only the directives, not the opcodes) would be useful.

There are some possibly useful attachments here:
http://bugzilla.gnome.org/show_bug.cgi?id=152267

I hope that others will comment on the patch contents.

-- 
Regards,
Pavel Roskin