c ++ shell cho 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;

}

Tôi có một trình bao mà tôi đã viết mã bằng c ++ trên bản phân phối Fedora Linux. Tôi hoan nghênh các cải tiến chung về cách làm cho mã tốt hơn, nhưng tôi đặc biệt hoan nghênh các nhận xét về khả năng đọc của mã

Answer 1

Có một số cải tiến bạn có thể thực hiện đối với mã này chỉ bằng cách sử dụng các hàm và lớp thư viện chuẩn c ++.

1. Không sử dụng #include <bits/stdc++.h>

Nó không đảm bảo rằng tệp tiêu đề này tồn tại và là một trình biên dịch nội bộ cụ thể. Sử dụng như vậy sẽ làm cho mã của bạn ít di động hơn.
Chỉ #includecác tiêu đề được cung cấp cho các lớp và hàm bạn muốn sử dụng từ thư viện chuẩn c ++.
Bạn có thể đọc thêm về các hậu quả và sự cố có thể xảy ra tại đây: Tại sao tôi không nên #include ?

Đồng thời #include, không sử dụng các tệp tiêu đề mà bạn không sử dụng bất kỳ thứ gì từ chúng (ví dụ #include <filesystem>).

2. Không sử dụng các hàm thư viện c cho các thao tác chuỗi

Ví dụ: mã của bạn để xây dựng promptbiến có thể được đơn giản hóa đáng kể bằng cách chỉ sử dụng std::stringthay vì char*:

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

Ngoài ra bạn có thể viết đơn giản

if(command == "quit"){

giả sử bạn sử dụng const std::string&làm loại cho commandtham số.

3. Bạn không cần phải cấp phát mảng char*biến để chuyển chúng cho execxy()các hàm

Chỉ cần xây dựng một std::vector<const char*>thay vì conv()chức năng của bạn :

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);
  }
}

Trong trường hợp bạn sử dụng con trỏ dữ liệu thô thu được bằng ví dụ std::string::data(), hãy đảm bảo rằng vòng đời của các biến cơ bản kéo dài trong suốt quá trình sử dụng chúng trong các hàm thư viện C chẳng hạn.

Nguyên tắc chung:
Tránh tự mình quản lý bộ nhớ bằng cách sử dụng newvà deleterõ ràng. Thay vì sử dụng vùng chứa tiêu chuẩn c ++ hoặc ít nhất là con trỏ thông minh .

4. Bạn không cần so sánh rõ ràng cho boolcác giá trị

Thay đổi

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

đến

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

Cũng sử dụng falsevà truethay vì các chuyển đổi ngầm từ int 0và nghĩa 1đen.

5. Sử dụng constvà chuyển bằng tham chiếu cho các tham số bất cứ khi nào có thể

Sử dụng constnếu bạn không cần thay đổi thông số.
Sử dụng pass by reference ( &) nếu bạn muốn tránh các bản sao không cần thiết được tạo cho các loại không tầm thường.

Bạn có thể xem cách thực hiện trong ví dụ Execute()tôi đã đưa ra ở trên.

Ví dụ tương tự

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

điều này nên

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

Answer 2

Định dạng.

Đây là một bức tường lớn của văn bản. Bạn cần chia mọi thứ thành các phần hợp lý để làm cho phần này dễ đọc hơn. Thêm một số khoảng trống dọc giữa phần để làm cho phần này dễ đọc hơn.

---

Bạn có một loạt #include. Thật tuyệt khi đặt hàng chúng. Bạn có thể chọn bất kỳ cách nào để sắp xếp nó miễn là hợp lý và giúp mọi người dễ dàng xem qua.

Tôi làm cụ thể nhất cho hầu hết chung chung.

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

Những người khác liệt kê chúng theo thứ tự bảng chữ cái.

Không chắc điều gì là tốt nhất nhưng một số logic để sắp xếp sẽ rất tốt.

---

Điều này thực sự khó đọc. Tôi không thể nhìn thấy tên hàm trong biển văn bản.

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);

Với một số cách sử dụng thận trọng usingvà một số ngăn nắp , bạn có thể làm cho nó thực sự dễ sử dụng.

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);

Mã

"Biến" toàn cầu là một ý tưởng tồi.

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

Thiết lập điều này trong main(). Sau đó, bạn có thể chuyển chúng dưới dạng tham số hoặc thêm chúng vào một đối tượng.

Bạn có thể có trạng thái bất biến tĩnh trong phạm vi toàn cầu. Điều này dành cho những thứ như hằng số.

---

Làm cho nó dễ đọc.

  while (1) {

Điều này sẽ tốt hơn như:

  while(true) {

---

Không sử dụng bộ đệm có kích thước cố định nơi người dùng có thể nhập các chuỗi có độ dài tùy ý. C ++ có khả std::stringnăng xử lý loại tình huống này.

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

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

---

Không sử dụng số ma thuật trong mã của bạn:

    char prompt[110] = "$[";

Tại sao lại là 110? Đặt các số ma thuật vào các hằng số được đặt tên

    // 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];

---

Nên sử dụng std :: string ở đây

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

Các hàm chuỗi C cũ không an toàn.

---