суббота, 30 мая 2015 г.

[prog.flame] Забавно, когда в качестве доказательства приводят неважно написанный C-шный код

В обсуждении релиза SObjectizer-5.5.5 на LOR-е в очередной раз всплыла идея о том, что приложение на акторах нужно писать на Erlang-е, а не на C++. И уж если производительности будет недостаточно, то тогда переписать узкие места на C в виде NIF-ов/драйверов:

Я и предложил нивелировать разрыв между C++, в данном случае, и чистым Erlang'ом с помощью NIF'ок, драйверов и пр. Ибо это элементарно проще, а по производительности будет идти ноздря в ноздрю с С++.

Идея не новая. Одна из вещей, которая серьезно портит эту идею, состоит в том, что писать нормальный и корректный код на C не просто. Как по мне, как гораздо сложнее, чем на C++. Ну да суть не столько в том, что C++ удобнее для написания надежного софта, чем C. А в том, какого качества C-шный код будут производить на свет обычные Erlang-еры, которые C видят лишь время от времени?

Собственно, приведенный на LOR-е код является тому подтверждением...

Позволю себе скопипастить этот пример сюда, дабы не нужно было бегать на другой сайт:

static ERL_NIF_TERM open_card(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) {
    UNUSED(argc);

    if (!asound_res)
        asound_res = enif_open_resource_type(env, NULL"asound_res", asound_handle_dtor, ERL_NIF_RT_CREATE | ERL_NIF_RT_TAKEOVER, NULL);

    char card_name[255] = {0};
    int mode = -1;

    if (enif_get_string(env, argv[0], card_name, 255, ERL_NIF_LATIN1) < 0)
        return enif_make_badarg(env);

    if (!enif_get_int(env, argv[1], &mode) || mode > SND_PCM_STREAM_LAST || mode < 0)
        return enif_make_badarg(env);

    asound_handle *h = (asound_handle*)enif_alloc_resource(asound_res, sizeof(asound_handle));
    bzero(h, sizeof(asound_handle));

    if (snd_pcm_open(&h->handle, card_name, mode, 0) < 0) {
        enif_release_resource(h);

        return enif_make_badarg(env);
    }
    snd_pcm_hw_params_malloc(&h->params);
    snd_pcm_hw_params_any(h->handle, h->params);
    snd_pcm_hw_params_set_access(h->handle, h->params, SND_PCM_ACCESS_RW_INTERLEAVED);

    ERL_NIF_TERM res = enif_make_resource(env, (void*)h);

    enif_self(env, &h->owner);
    h->env  = enif_alloc_env();
    h->port = enif_make_copy(h->env, res);
    h->mode = mode;
    h->recording = 0;

    enif_release_resource(h);

    return enif_make_tuple2(env, enif_make_atom(env, "ok"), res);
}

....


/** ALSA set period time */
static ERL_NIF_TERM set_period_time(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) {
    UNUSED(argc);

    asound_handle *h = NULL;
    unsigned val = 0;
    int dir = 0;

    if (!enif_get_resource(env, argv[0], asound_res, (void**)&h))
        return enif_make_badarg(env);
    else if (!enif_get_int(env, argv[1], (int*)&val))
        return enif_make_badarg(env);

    snd_pcm_hw_params_set_period_time_near(h->handle, h->params, &val, &dir);

    return enif_make_tuple2(env, enif_make_atom(env, "ok"), enif_make_int(env, val));
}

Меня могут спросить, в чем я здесь вижу проблему?

Если вы так спрашиваете, значит вы и сами будете писать в таком же духе. Возможно, вас качество этого кода устраивает. И, вполне возможно, в 99% случаев с эти кодом не будет никаких проблем. Так что для многих качество этого кода можно охарактеризовать как "good enough".

Тем не менее, будь автором этого кода кто-то из моих подчиненных, я бы заставил исправить ошибки.

Ошибки здесь связаны с игнорированием кодов возврата функций snd_pcm_hw_params_*. В первую очередь это касается контроля за результатом snd_pcm_hw_params_malloc. Если память выделить не удалось, то как можно спокойно продолжать работать?

Но и во второй NIF-ной функции, set_period_time, результат вызова snd_pcm_hw_params_set_period_time_near тоже не мешало бы проверить. А то ведь оказывается, что возвращаемое значение set_period_time вообще (ну т.е. совсем) не зависит от того, как отработала set_period_time_near -- всегда возвращается тупл из атома Ok + какой-то int, возможно, переданный в качестве параметра.

Исправления до тривиальности простые. Нужно просто оборачивать все вызовы snd_pcm_hw_params_* в if-ы. И предпринимать соответствующие действия, если возвращается код ошибки.

Очень просто. Но код тогда превратиться в месиво из if-ов и return-ов (или if-ов и goto err). Да и писать в таком стиле смогут не многие -- это уже неоднократно доказано практикой.

Так что в теории, может быть, написание производительного софта на Erlang-е с переписыванием узких мест на C и выглядит красиво. Но на практике там будет столько посредственного качества кода с таким количеством ждущих своего времени ошибок, что мало не покажется.

PS. Разбираясь с этим кодом пришлось заглянуть в исходники Erlang-а, чтобы понять, как ведет себя enif_alloc_resource в случае нехватки памяти. Оказалось, что прерывает работу приложения. Суровый подход, однако. Впрочем, что-то он мне напоминает :)

Отправить комментарий