[haiku-commits] Re: haiku: hrev44828 - src/system/boot/arch/arm headers/private/kernel/arch/arm
- From: Axel DÃrfler <axeld@xxxxxxxxxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Fri, 16 Nov 2012 18:09:08 +0100
Am 13/11/2012 12:09, schrieb ithamar.adema@xxxxxxxxxxxxxxxx:
+/*! check_cpu_features
+ *
+ * Please note the fact that ARM7 and ARMv7 are two different things ;)
+ * ARMx is a specific ARM CPU core instance, while ARMvX refers to the
+ * ARM architecture specification version....
+ *
+ * Most of the architecture versions we're detecting here we will probably
+ * never run on, just included for completeness sake... ARMv5 and up are
+ * the likely ones for us to support (as they all have some kind of MMU).
+ */
Just pointing out a few style issues:
- The function name does not need to be duplicated in the comment
- For function documentation, we usually use the following format:
/*! This function is meaningless.
I only wrote it because I can.
*/
int
path_to_greatness(int me)
{
return me + 1;
}
Note that there is no blank line between the doxygen comment and the
function.
+ uint32 result = 0;
+ int arch, variant = 0, part = 0, revision = 0, implementor = 0;
Prefer to put each on its own line.
+ switch((result >> 12) & 0xf) {
Space between 'switch' and '('.
-
// #pragma mark -
-
Two blank lines.
Also, from hrev44822:
> +++ b/src/add-ons/kernel/drivers/disk/norflash/norflash.cpp
> + * Authors:
> + * Ithamar R. Adema <ithamar@xxxxxxxxxxxxxxxxxxx>
> + */
> +
> +#include <drivers/device_manager.h>
Two blank lines.
> +#include <drivers/KernelExport.h>
> +#include <drivers/Drivers.h>
> +#include <kernel/OS.h>
> +
> +#include <string.h>
> +#include <stdlib.h>
Include order is most generic to most specific so the POSIX headers come
first. Only the file's counterpart header should be first to make sure
the header is self contained.
> +static device_manager_info* sDeviceManager;
> +
> +static const char *sTabTab = "\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t";
> +#define DS "%.*s"
> +#define DA depth - 1, sTabTab
> +
> +static void
Two blank lines between sections, mix of asterisk style. DS/DA aren't
good names (but I assume this code will be removed anyway again).
> +static status_t
> +nor_open(void *deviceCookie, const char *path, int openMode,
> + void **_cookie)
The second line should only be indented by a single tab.
> +static status_t
> +nor_ioctl(void *cookie, uint32 op, void *buffer, size_t length)
> +{
> + nor_driver_info* info = (nor_driver_info*)cookie;
> + TRACE("ioctl(%ld,%lu)\n", op, length);
> +
> + switch(op) {
> + case B_GET_GEOMETRY:
> + {
Space after switch, case is indented.
Bye,
Axel.
Other related posts:
