[EMAIL PROTECTED] dixit:
Willst Du richtig schön harte Kritik?
> trunk/freewrt/tools/nfotizer/
-ize ist amerikanisch, -ise ist englisch.
>+CFLAGS?= -Wall
>+CC?= gcc
Vorgegeben vom Buildsystem.
>+all: $(TARGET)
Wir wollten geschweifte Klammern für Variablen nehmen.
>+%.o: %.c $(HEADERS)
>+ $(CC) $(CFLAGS) -c $<
Redundant, das ist ne implizite automatische Regel.
>+char *ipkg[] = { "#", "#NAME#.control",
char * impliziert die Schreibbarkeit, auch für die Elemente
eines char * [], aber ein "#" ist ein const char[] (oder ein
const char *), daher geht das so nicht.
>+#include "nfotizer.h"
Bitte keine Headerdateien andere Headerdateien einbinden lassen.
>+htab *htab_create(int size)
Gemäß http://www.mirbsd.org/man9/style.htm schreibst Du besser
htab *
htab_create(int size)
also in zwei Zeilen. Selbst Linus findet das gut, auch wenns
das nicht mehr in den Linux-Kernel Coding Style geschafft hat.
>+ int i=0;
^^
>+ for(;i<tab.items;i++) {
^ ^^^ ^
Leerzeichen tun Not - zumindest hinter Keywords (wenn Du nicht
weißt, was alles ein Keyword ist, guck im K&R nach, oder unter
entweder http://www.mirbsd.org/cman/manPSD/06.Clang.htm oder
http://www.mirbsd.org/cman/manPSD/23.Cknr.htm - sind beide re-
lativ objektiv).
>+ return -1;
style(9) - s.o. - sagt: bitte Klammern drum. Ist allerdings
nicht so schlimm wie der Rest.
>+ //debug("inserted item: %s: %s\n", tab->data[tab->items].key,
>tab->data[tab->items].val);
C99 C++-style Comments sind böse. Nimm ein Makro:
#ifdef DEBUG
#define debug(foo, ...) fprintf(stderr, foo, ##__VA_ARGS__)
#else
#define debug(foo, ...) /* nothing */
#enif
>+#define TOGGLE(x) ( x = ((x==1) ? 0 : 1) )
Uh, was ist an '~x' verkehrt? Übrigens nimmt man für sowas
schlicht und ergreifend <stdbool.h>.
>+/* allocating strcat */
>+char *my_strcat(char *dest, char *src)
>+{
>+ int newlen = strlen(dest) + strlen(src) + 1;
>+ char *ret = calloc(newlen, sizeof(char));
>+ if(sprintf(ret, "%s%s", dest, src) < 0)
ARRGH! Grausam!
Schon mal sowas probiert? Statt
strcpy(foo, bar);
strcat(foo, baz);
was eh unsicher ist (strcpy/strcat sind absolut(!) tabu, strncpy will man
nicht, weil die API von strncat scheiße ist, also bleibt nur strlcpy und
strlcat, welche übrigens in uClibc drin sind, oder eben):
asprintf(&foo, "%s%s", bar, baz);
was sogar alloziert. http://www.mirbsd.org/man3/asprintf.htm FWIW.
>+{
>+ int myfmt_len = strlen(fmt);
>+ char *myfmt = calloc(myfmt_len, sizeof(char));
>+ char *fullstr = strdup(fmt);
>+ char *result, *token;
>+ int is_id = (*fmt == *idid) ? 1 : 0;
Initialisierungen mit nichtkonstanten Argumenten (strlen, calloc, strdup) sind
richtig richtig böse und können, je nach Compiler und Optimierungen, total in
die Hose gehen. is_id sollte zwar eh besser ein bool - cf. <stdbool.h> - sein,
aber die Initialisierung ist fast safe - fast weil fmt und idid NULL sein
könnten.
>+char *my_dirname(char *path)
>+{
[…]
>+ return "";
>+}
Eine Funktion, die „char *“ zurückgibt, kann doch nicht „(const char *)""“
zurückgeben! Der Aufrufer könnte versuchen, in das Resultat zu schreiben!
>+#ifdef DEBUG
>+ printf("\nItems in table:\n");
Debugging geht bitte nach stderr.
>+ if(key) free(key);
Newlines are for the weak.
>+ case '\n':
>+ // FALL THROUGH
>+ case '#':
http://www.mirbsd.org/man1/lint.htm parst „/* FALLTHRU */” oder
„/* FALLTHROUGH */“. Wie bereits erwähnt sind die Doppelschrägstrich-
Kommentare böse, die nimmt man, um kurz zum Testen was auszukommentieren,
aber das wars dann auch, für Kommentate /*…*/ und für Debugging #if 0…#endif.
So, und das war erst der Anfang…
Gruß
//mirabile (nach 3 Bier, Wein, Julishka, etc. aber trotzdem noch ernst)
PS: Wenns mit „-Wall -Wextra -Wunused -Wdeclaration-after-statement -Wundef
-Wendif-labels -Wshadow -Wpointer-arith -Wbad-function-cast
-Wcast-qual -Wcast-align -Wwrite-strings -Wstrict-prototypes
-Wold-style-definition -Wmissing-prototypes -Winline -Werror
-Wmissing-declarations -Wmissing-noreturn -pedantic -std=gnu99
-Wmissing-format-attribute -Wredundant-decls“ baut, bist Du gut, auch
wenn das nur die “most basic of errors” abfängt. (Diese Warnungen tun
mit gcc 3.4.x - bei anderen Versionen halt anpassen…)
--
I believe no one can invent an algorithm. One just happens to hit upon it
when God enlightens him. Or only God invents algorithms, we merely copy them.
If you don't believe in God, just consider God as Nature if you won't deny
existence. -- Coywolf Qi Hunt
_______________________________________________
freewrt-developers mailing list
[email protected]
https://www.freewrt.org/lists/listinfo/freewrt-developers
