Re: [Lost] [Patch] [1/4] Service-Manager

[Kevin Wolf <mail@xxxxxxxxxxxxx>]

Am Donnerstag, 26. Juni 2008 22:46:59 schrieb Antoine Kaufmann:
> Hier eine korrigierte Version des Service-Managers

> +/**
> + * Konfigurationsverzeichnis parsen
> + *
> + * @param dir Verzeichnis-Handle
> + *
> + * @return TRUE wenn erfolgreich abgeschlossen wurde, FALSE sonst.
> + */
> +static bool config_dir_parse(io_resource_t* dir)
> +{
> +    io_direntry_t* entry;
> +    struct config_service* conf_serv;
> +    int i;
> +
> +    config_list = list_create();
> +
> +    // In einem ersten Schritt werden alle Servicenamen eingelesen
> +    while ((entry = directory_read(dir))) {
> +        // Alles was nicht Verzeichnis ist, wird uebersprungen
> +        if ((entry->type != IO_DIRENTRY_DIR) || (entry->name[1] == '.')) {
> +            free(entry);
> +            continue;
> +        }
> +
> +        // Service in der Liste eintragen
> +        conf_serv = malloc(sizeof(*conf_serv));
> +        assert(conf_serv != NULL);

Wie wäre es mit calloc? Hier wird ja nicht alles an einer Stelle 
initialisiert, da besteht erst recht Gefahr, was zu vergessen.

> +
> +        // Namen setzen
> +        assert(asprintf(&conf_serv->name, "%s", entry->name) != -1);

Die assert-Bedingung darf nie Seiteneffekte haben.

> +
> +        list_push(config_list, conf_serv);
> +
> +        free(entry);
> +    }
> +
> +    // In einem zweiten schritt werden die Befehle zum Starten und die
> +    // Abhaengigkeiten eingetragen
> +    for (i = 0; (conf_serv = list_get_element_at(config_list, i)); i++) {
> +        char* cmd_path;
> +        char* deps_path;
> +        char* conf_path;
> +
> +        assert(asprintf(&cmd_path, "%s/%s/cmd", full_config_path,
> conf_serv-> +            name) != -1);
> +        assert(asprintf(&deps_path, "%s/%s/deps", full_config_path,
> conf_serv-> +            name) != -1);
> +        assert(asprintf(&conf_path, "%s/%s/conf", full_config_path,
> +            conf_serv->name) != -1);

Siehe oben.

> +
> +        config_read_file(cmd_path, &conf_serv->start_cmd);
> +        config_parse_deps(deps_path, conf_serv);
> +        config_parse_conf(conf_path, conf_serv);
> +        free(deps_path);
> +        free(cmd_path);

conf_path wird nicht freigegeben?

> +    }
> +
> +    return TRUE;
> +}
> +
> +
> +bool config_read()
> +{
> +    const char* config_path = "/config/servmgr";
> +    io_resource_t* dir;
> +    bool result;
> +
> +    assert(asprintf(&full_config_path, "%s%s", root_dir, config_path) !=
> -1); +

Siehe oben.

> +    // Verzeichnis mit der Konfiguration oeffnen
> +    dir = directory_open(full_config_path);
> +    if (!dir) {
> +        printf("servmgr: Konnte Konfigurationsverzeichnis nicht oeffnen:
> '%s'", +            full_config_path);

Ein \n vielleicht?

> +/**
> + * Konfiguration eines Dienstes
> + */
> +struct config_service {
> +    char* name;
> +    char* start_cmd;
> +    list_t* deps;
> +
> +    struct {
> +        bool no_wait;
> +    } conf;
> +};

Die struct-Definition würde ich an den Anfang der Headerdatei stellen und erst 
hinterher die ganzen Funktionen. Aber das ist wohl Geschmackssache.