From: Daniel Stenberg <daniel_at_haxx.se>
Date: Sat, 1 Feb 2003 13:34:13 +0100 (CET)

On Fri, 31 Jan 2003 rockbox_at_diffenbach.org wrote:

> I'm also interested in having my code critiqued for compliance with
> "best-practice" Rockbox code;

I'll focus my criticism on this topic. I'm a Linux user and I don't have any
.lnk files lying around to test. I do appreciate your efforts, please take my
nits as friendly remarks on how to make your code better fit in with all the
existing code.

General formatting rules:

* We use plain old /* C comments */ and not // C++ comments
* Don't use tabs in source code.
* Indentation level is 4 spaces
* Please don't write source code that is wider than 79 columns or so

> typedef long dword ;
> typedef dword qword[ 2 ] ;

I'm not sure if you intend to keep these things, but in Rockbox we try to
avoid unnecessary typecasts of this kind that adds nothing but confusion.

Use 'long' instead of 'dword' and the qword can just be made a long array
with two entries.

> // end header
>
> #include <fcntl.h>
> #include<io.h>
> #include <stdio.h>
> #include <string.h>

We usually put all the include files at the top of the file.

> struct lnk_header_and_next_length header ;

...

> if( ( ret = read( file_handle, &header, sizeof( header ) ) ) != sizeof(
> header ) )
> return LNK_READ_ERROR ;

This kind of code I *think* will require that the struct is defined with
__attribute__((packed)) in gcc (see tools/bmp2rb.c) to work fine
crossplatform etc.

Again, I have not tested this or even attempted to follow the logic of the
code itself.

-- 
 Daniel Stenberg -- http://rockbox.haxx.se/ -- http://daniel.haxx.se/

Received on 2003-02-01