Насколько отвратителен данный код?

Question

[Twelfth Doctor]

Здравствуйте. Насколько отвратителен данный код? На работоспособность пока что не проверял, просто хотелось бы узнать Ваше мнение.

bool update_uwsgi_config() {
         const char *source = fmt::format("/tmp/{}.ini", unid).c_str();
         const char *on_restart = "service uwsgi restart";
         const char *dest = "/etc/uwsgi/apps-available/www.ini";
         const char *on_update = fmt::format("lxc file push {} {} {}", source,contname,dest).c_str();
         system(on_update);
         system(on_restart);
         remove(dest);
         return true;
     }

Answer 1

[Mercury13]

Главная проблема.

const char *source = fmt::format("/tmp/{}.ini", unid).c_str();

Объект fmt::format сразу же исчезнет, и source будет «висеть».

Придётся писать…

fmt::format fmSource("/tmp/{}.ini", unid);
const char *source = fmSource.c_str();

И так далее.

Comments

[Евгений Шатунов]

Mercury13 , сделаю одну поправку. fmt - это очень удобная библиотека для форматирования. :)
https://github.com/fmtlib/fmt

fmt::format возвращает строку (std::string), которая все равно грохнется там, где ты описал.

[Twelfth Doctor]

Евгений Шатунов, а как сделать так, чтобы она не грохнулась?

[Евгений Шатунов]

Twelfth Doctor , каждая строчка кода должна быть технически обоснована.
В виду какой необходимости ты использовал тип const char* для своих переменных вместо std::string?

Если я просто скажу что тебе надо использовать тип std::string вместо const char*, то большая часть проблемы так и останется нерешенной.
Тебе самому надо обосновать это использование, т.е. понять, почему так стоит делать.

И, опять же, std::string использовать стоит далеко не всегда. В ряде случаев использование будет избыточным. В приведенном тобой коде тоже есть такой случай.
Поэтому и нужно обосновывать каждую строчку своего кода.

[Mercury13]

Евгений Шатунов, Но я был не против fmt::format. Я был против ПРЕЖДЕВРЕМЕННО ИСЧЕЗНУВШЕГО объекта fmt::format.

[Евгений Шатунов]

Mercury13 , в fmtlib нет такого объекта. fmt::format - это функция. :)

[Twelfth Doctor]

В виду какой необходимости ты использовал тип const char* для своих переменных вместо std::string?

Евгений Шатунов, потому что int remove ( const char * filename );
В функцию remove передаётся именно const char
И в функцию system - тоже
int system(const char *command);

[Twelfth Doctor]

#include <iostream>
using namespace std;

void main() {
	string name = "ls -l";
	system(name);
}

Данная программа не будет работать

prog.cpp:6:13: error: cannot convert ‘std::__cxx11::string {aka std::__cxx11::basic_string<char>}’ to ‘const char*’ for argument ‘1’ to ‘int system(const char*)’
  system(name)

[Евгений Шатунов]

Twelfth Doctor , remove и system никак не обязывают тебя использовать какой-либо тип для своих переменных.

Пример кода хороший. Простой и понятный.
Скажи, мешает ли тебе что-нибудь исправить твой пример кода вот так?

#include <iostream>
using namespace std;

void main() {
  string name = "ls -l";
  system(name.c_str());
}

std::string - это класс, у него есть довольно удобный интерфейс. Благодаря этому интерфейсу ты можешь куда легче оперировать со строками. Сам std::string реализует двойную схему управления памятью. Короткие строки он может хранить в себе, а длинные - в динамической памяти. Это снимает с тебя тяжбы слежения за памятью и позволяет думать только в рамках твоей задачи.

Answer 2

[xSkyFoXx]

1) Предпочтительнее использовать string из std, в этом случаи вам не надо заниматься явным приведением типов.
2) Если вы используете константы для всего приложения, перенесите их на уровень вашей текущей единицы трансляции. Снабдите коментариями для ясности.
3) fmt::format здесь лишняя зависимость. Вы можете воспользоваться сразу Си-строками, или сформировать строку через конкатинацию.
4) Если вы всегда возвращаете true, тогда зачем вообще что-либо возвращать? void должно быть достаточно хорошо.

Answer 3

[devalone]

- Не кросплатформерно(хотя вряд ли она тут нужна, если используется lxc)
- если contname или unid принимаются от пользователя, то это серьёзная уязвимость
- зачем лишняя зависимость, если форматирование есть в std::?
- константы лучше выносить в конфиг, удобнее редактировать потом

Да и такое обычно пишут на python/bash, если конечно это не часть проекта, который уже на C++