shell c ++ para linux

Question

#include <cstring>
#include <map>
#include <iostream>
#include <fstream>
#include <sstream>
#include <string>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
#include <vector>
#include <filesystem>
#include <errno.h>
#include <bits/stdc++.h>
std::string USERDIR = getenv("HOME");
std::string ALIASFILE = USERDIR+"/shell/.alias";
std::vector<std::string> Split(std::string input, char delim);
void Execute(const char *command, char *arglist[]);
std::map<std::string, std::string> alias(std::string file);
bool BuiltInCom(const char *command, char *arglist[],int arglist_size);
char** conv(std::vector<std::string> source);
bool createAlias(std::string first, std::string sec);
std::string replaceAll(std::string data, std::map <std::string, std::string> dict);
int main() {
  while (1) {
    char path[100];
    getcwd(path, 100);
    char prompt[110] = "$[";
    strcat(prompt, path);
    strcat(prompt,"]: ");
    std::cout << prompt;
    // Takes input and splits it by space
    std::string input;
    getline(std::cin, input);
    if(input == "") continue;
    std::map<std::string, std::string> aliasDict = alias(ALIASFILE);
    input = replaceAll(input, aliasDict);
    std::vector<std::string> parsed_string = Split(input, ' ');
    // Splits parsed_string into command and arglist
    const char * com = parsed_string.front().c_str();
    char ** arglist = conv(parsed_string);
    // Checks if it is a built in command and if not, execute it
    if(BuiltInCom(com, arglist, parsed_string.size()) == 0){
        Execute(com, arglist);
    }
    delete[] arglist;
  }
}

std::vector<std::string> Split(std::string input, char delim) {
  std::vector<std::string> ret;
  std::istringstream f(input);
  std::string s;
  while (getline(f, s, delim)) {
    ret.push_back(s);
  }
  return ret;
}

void Execute(const char *command, char *arglist[]) {
  pid_t pid;
  //Creates a new proccess
  if ((pid = fork()) < 0) {
    std::cout << "Error: Cannot create new process" << std::endl;
    exit(-1);
  } else if (pid == 0) {
    //Executes the command
    if (execvp(command, arglist) < 0) {
      std::cout << "Could not execute command" << std::endl;
      exit(-1);
    } else {
      sleep(2);
    }
  }
  //Waits for command to finish
  if (waitpid(pid, NULL, 0) != pid) {
    std::cout << "Error: waitpid()";
    exit(-1);
  }
}

bool BuiltInCom(const char *command, char ** arglist, int arglist_size){
  if(strcmp(command, "quit") == 0){
    delete[] arglist;
    exit(0);
  } else if(strcmp(command, "cd") == 0){
    if(chdir(arglist[1]) < 0){
      switch(errno){
        case EACCES:
          std::cout << "Search permission denied." << std::endl;
          break;
        case EFAULT:
          std::cout << "Path points outside accesable adress space" << std::endl;
          break;
        case EIO:
          std::cout << "IO error" << std::endl;
          break;
        case ELOOP:
          std::cout << "Too many symbolic loops" << std::endl;
          break;
        case ENAMETOOLONG:
          std::cout << "Path is too long" << std::endl;
          break;
        case ENOENT:
          std::cout << "Path doesn't exist" << std::endl;
          break;
        case ENOTDIR:
          std::cout << "Path isn't a dir" << std::endl;
          break;

        default:
            std::cout << "Unknown error" << std::endl;
            break;
      }
      return 1;
    }
    return 1;
  } else if(strcmp(command, "alias") == 0){
    if(arglist_size < 2){
      std::cout << "[USAGE] Alias originalName:substituteName" << std::endl;
      return 1;
    }
    std::string strArg(arglist[1]);
    int numOfSpaces = std::count(strArg.begin(), strArg.end(), ':');
    if(numOfSpaces){
      std::vector<std::string> aliasPair = Split(strArg, ':');
      createAlias(aliasPair.at(0), aliasPair.at(1));
      return 1;
    } else {
      std::cout << "[USAGE] Alias originalName:substituteName" << std::endl;
      return 1;
    }
  }
  return 0;
}

char** conv(std::vector<std::string> source){
  char ** dest = new char*[source.size() + 1];
  for(int i = 0; i < source.size(); i++) dest[i] = (char *)source.at(i).c_str();
  dest[source.size()] = NULL;
  return dest;
}


std::map<std::string, std::string> alias(std::string file){
  std::map<std::string, std::string> aliasPair;
  std::string line;
  std::ifstream aliasFile;
  aliasFile.open(file);
  if(aliasFile.is_open()){
    while(getline(aliasFile, line)){
      auto pair = Split(line, ':');
      aliasPair.insert(std::make_pair(pair.at(0), pair.at(1)));
    }
  } else {
    std::cout << "Error: Cannot open alias file\n";
  }
  return aliasPair;
}
std::string replaceAll(std::string data, std::map <std::string, std::string> dict){
  for(std::pair <std::string, std::string> entry : dict){
      size_t start_pos = data.find(entry.first);
      while(start_pos != std::string::npos){
        data.replace(start_pos, entry.first.length(),entry.second);
        start_pos = data.find(entry.first, start_pos + entry.second.size());
      }

  }
  return data;
}
bool createAlias(std::string first, std::string second){
    std::ofstream aliasFile;
    aliasFile.open(ALIASFILE, std::ios_base::app);
    if(aliasFile.is_open()){
      aliasFile << first << ":"<< second << std::endl;
      return true;
    } else return false;

}

Eu tenho um shell que codifiquei em c ++ em uma distribuição do Fedora Linux. Gostaria de receber melhorias gerais sobre como tornar o código melhor, mas gostaria especialmente de comentários sobre a legibilidade do código

Answer 1

Existem várias melhorias que você pode fazer para este código usando apenas classes e funções da biblioteca padrão c ++.

1. Não use #include <bits/stdc++.h>

Não é garantido que este arquivo de cabeçalho exista e seja um compilador interno específico. Usar isso tornará seu código menos portátil.
Apenas #includecabeçalhos fornecidos para as classes e funções que você deseja usar da biblioteca padrão c ++.
Você pode ler mais sobre as possíveis consequências e problemas aqui: Por que eu não deveria #include ?

Da mesma forma, #includenão use arquivos de cabeçalho onde você não usa nada deles (por exemplo #include <filesystem>).

2. Não use funções da biblioteca c para manipulações de strings

Por exemplo, seu código para construir a promptvariável pode ser drasticamente simplificado apenas usando em std::stringvez de char*:

char path[100];
getcwd(path,100);
std::string prompt = "$[" + std::string(path) + "]:";

Além disso, você pode simplesmente escrever

if(command == "quit"){

suponho que você use const std::string&como tipo para o commandparâmetro.

3. Você não precisa alocar matrizes de char*variáveis ​​para passá-las para execxy()funções

Acabei de criar um em std::vector<const char*>vez de sua conv()função:

void Execute(const std::string& command, const std::vector<std::string>& args) {
  std::vector<const char*> cargs;
  pid_t pid;

  for(auto sarg : args) {
      cargs.append(sarg.data());
  }
  cargs.append(nullptr);

  //Creates a new proccess
  if ((pid = fork()) < 0) {
    std::cout << "Error: Cannot create new process" << std::endl;
    exit(-1);
  } else if (pid == 0) {
    //Executes the command
    if (execvp(command.data(), cargs.data()) < 0) {
      std::cout << "Could not execute command" << std::endl;
      exit(-1);
    } else {
      sleep(2);
    }
  }
  //Waits for command to finish
  if (waitpid(pid, NULL, 0) != pid) {
    std::cout << "Error: waitpid()";
    exit(-1);
  }
}

Nesse caso, onde você usa ponteiros de dados brutos obtidos, por exemplo std::string::data(), certifique-se de que os tempos de vida das variáveis ​​subjacentes durem ao longo de seu uso, por exemplo, funções de biblioteca C

Como regra geral:
evite fazer você mesmo o gerenciamento de memória usando newe deleteexplicitamente. Em vez disso, use um contêiner padrão c ++ ou, pelo menos, ponteiros inteligentes .

4. Você não precisa de comparação explícita para boolvalores

mudança

if(BuiltInCom(com, arglist, parsed_string.size()) == 0){

para

if(!BuiltInCom(com, arglist, parsed_string.size())){

Use também falsee em truevez das conversões implícitas de int 0e 1literais.

5. Use conste passe por referência para parâmetros sempre que possível

Use constse você não precisar alterar o parâmetro.
Use passar por referência ( &) se quiser evitar cópias desnecessárias feitas para tipos não triviais.

Você pode ver como no exemplo Execute()que dei acima.

O mesmo vale por exemplo

std::string replaceAll(std::string data, std::map <std::string, std::string> dict);

isso deve ser

std::string& replaceAll(std::string& data, const std::map <std::string, std::string>& dict);

Answer 2

Formatação.

Esta é uma grande parede de texto. Você precisa dividir as coisas em seções lógicas para tornar a leitura mais fácil. Adicione algum espaço vertical entre as seções para facilitar a leitura.

---

Você tem um monte de #include. É bom encomendá-los. Você pode escolher qualquer forma de fazer o pedido, desde que seja lógico e facilite a visualização das pessoas.

Eu faço o mais específico ao mais geral.

 #include "HeaderFileForThisSource.h"
 #include "HeaderFileForOtherClassesInThisProject"
 ...
 #include <C++ Librries>
 ...
 #include <C Librries>
 ...
 #include <Standard C++ Header Files>
 ..
 #include <C standard Libraries>
 ...

Outros os listam em ordem alfabética.

Não tenho certeza do que é melhor, mas alguma lógica para o pedido seria bom.

---

Isso é realmente difícil de ler. Não consigo ver os nomes das funções no mar de texto.

std::vector<std::string> Split(std::string input, char delim);
void Execute(const char *command, char *arglist[]);
std::map<std::string, std::string> alias(std::string file);
bool BuiltInCom(const char *command, char *arglist[],int arglist_size);
char** conv(std::vector<std::string> source);
bool createAlias(std::string first, std::string sec);
std::string replaceAll(std::string data, std::map <std::string, std::string> dict);

Com um pouco de uso criterioso usinge alguma arrumação, você pode tornar isso realmente fácil de usar.

using  Store = std::vector<std::string>;
using  Map   = std::map<std::string, std::string>;
using  CPPtr = char**;

Store       Split(std::string input, char delim);
void        Execute(const char *command, char *arglist[]);
Map         alias(std::string file);
bool        BuiltInCom(const char *command, char *arglist[],int arglist_size);
CPPtr       conv(std::vector<std::string> source);
bool        createAlias(std::string first, std::string sec);
std::string replaceAll(std::string data, std::map <std::string, std::string> dict);

Código

As "variáveis" globais são uma má ideia.

std::string USERDIR = getenv("HOME");
std::string ALIASFILE = USERDIR+"/shell/.alias";

Configure isso em main(). Você pode então passá-los como parâmetros ou adicioná-los a um objeto.

Você pode ter um estado estático imutável no escopo global. Isso é para coisas como constantes.

---

Facilite a leitura.

  while (1) {

Isso seria melhor se:

  while(true) {

---

Não use buffers de tamanho fixo, onde o usuário pode inserir strings de comprimento arbitrário. C ++ tem a capacidade std::stringde lidar com esse tipo de situação.

    char path[100];
    getcwd(path, 100);

    // Rather
    std::string  path = std::filesystem::current_path().string();

---

Não use números mágicos em seu código:

    char prompt[110] = "$[";

Por que 110? Coloque os números mágicos em constantes nomeadas

    // Near the top of the programe with all other constants.
    // Then you can tune your program without having to search for the constants.
    static std::size_t constepxr bufferSize = 110;

    .....
    char buffer[bufferSize];

---

Deve usar std :: string aqui

    strcat(prompt, path);
    strcat(prompt,"]: ");

As funções antigas da string C não são seguras.

---