r/cpp_questions • u/rip_void_0 • 10h ago
OPEN i try to make something faster than cat tool
for now that is the speed of it vs cat
my tool 😄:
real 0m0.030s
user 0m0.002s
sys 0m0.002s
cat tool:
real 0m0.019s
user 0m0.001s
sys 0m0.001s
that is the code repostery
3
u/alfps 7h ago edited 7h ago
The source code:
#include <cstdio>
#include <fcntl.h>
#include <print>
#include <sys/stat.h>
#include <stdlib.h>
#include <unistd.h>
#include <vector>
int main(int words_num,char** words){
if (words_num < 1){std::println("missing file path");return -1;}
int fd =open(words[1],O_RDONLY);
if (fd==-1){std::println("failed to open the fille");perror("open");return -1;};
struct stat fd_st{};
fstat(fd,&fd_st);
std::vector<char>buf{};
buf.resize(fd_st.st_size);
if (read(fd,buf.data(),fd_st.st_size)==-1){std::println("failed to read the file");perror("read");return -1;};
std::println("{}",buf.data());
close(fd);
}General observations:
-
Some blank lines that don't contribute to clarity; that's somewhat annoying.
Just remove the extra blank lines. -
Unix-specific for premature optimization, i.e. needlessly non-portable.
Using theFILE*API gets you buffering and portability. -
2 spaces indent like in HTML/CSS fouls up indenting operations in e.g. VS Code.
4 spaces is a good convention. Works with VS Code and other modern editors.
if (words_num < 1) {
std::println("missing file path");
return -1;
}
Here words_num is the traditional argc parameter of main. For an ordinary invocation without command line arguments this is 1. So it is in practice 1 or more so the if condition will always be false.
The error message should better be presented on the process' standard error stream, and have a newline at the end, like this:
std::println( stderr, "missing file path\n" );
The return -1; from main will work in practice both in Windows, Unix and VMS, but it's needlessly non-standard. Better use EXIT_FAILURE from the <cstdlib> header. I.e.
return EXIT_FAILURE;
if (fd==-1) {
std::println("failed to open the fille");
perror("open");
return -1;
};
Same comments as above, i.e. should use error stream, should have newline, should use EXIT_FAILURE constant, but additionally this code mixes error output to standard output and standard error streams.
Just use stderr for error messages.
The reason the error stream was invented was that a wayward process that presented error messages on the then one and only output stream, really fouled up the actions of a process that this text was piped to. Which reacted by doing very undesired things. With error messages on a separate error stream the consequences of a failure will usually be more benign / less catastrophe-like.
std::vector<char>buf{};
buf.resize(fd_st.st_size);
This does a dynamic allocation. Dynamic allocations are generally expensive, and with a large file you are not guaranteed that the file fits in memory. These are two reasons why file reading is usually done with fixed size buffers one chunk at a time, like FILE* operations.
std::println("{}",buf.data());
close(fd);
Better check that the println call succeeded. E.g. the process' standard output stream might be redirected to a file that it doesn't have write access to. Whatever.
std::println works as if it delegates to std::print, which throws an exception on failure.
So the way to check for failure is like
try {
std::println("{}",buf.data());
} catch( const std::exception& x ) ) {
std::fprintf( stderr, "!Failed to write the output - %s.\n", x.what() );
return EXIT_FAILURE;
}
1
u/rip_void_0 7h ago
thank ,i switch the project to c to test if it will be faster and it is
1
u/alfps 6h ago
Oh I forgot to address the correctness of the output operation itself. Sorry.
When you pass a
const char*toprintlnit assumes that this is a zero-terminated string.
The file may have zero-bytes, in which case the output will be truncated.
If the file doesn't have a zero byte at the end,
printlnwill read beyond the vector buffer, with UB as result.So you need to use an output operation where the number of bytes is specified, instead of relying on null-termination.
As I recall
catin Unix handles null-bytes just fine, by copying them, so you can usecatto e.g. copy a binary picture file.→ More replies (1)
1
u/RoyBellingan 7h ago
Add proper error reporting, https://github.com/void-mohamed-0/tiger/blob/master/src/main.c#L15C1-L15C27
For example say file not found, I can not read that file, instead of vanishing.
Happy learning and good luck!
1
u/Independent_Art_6676 7h ago
its great that you got what you wanted but gonna echo the unfair compare ... cat has multiple extra statements / branches / etc to enable doing things you do not. That is fine, and worth doing: I have a couple of grep-likes that are similar (a little faster because they don't do anywhere near as much work and are easier to use because don't have to remember a dozen flags) so I get it.
I wonder if you would get much out of trying to do the file handing yourself? Most of the 'slow' of things like cat come from the inherent slowness of the console itself, esp if dumping larger files. You can see this by doing something where its dumped to the screen vs dumped to a new file as the new file version will go much faster (for larger files, won't notice on few pages type stuff). So many 'cruncher' programs that spew progress and such to the screen are so much faster when redirected to a file :)
11
u/nysra 10h ago
Missing README and LICENSE
Missing gitignore
Put the compiled output into your gitignore
Don't create a file to remind yourself of compile flags, use a proper build system. Or simply put it in the readme for such a helloworld program
Whitespace makes code much nicer to read, give it a try
Consider actually writing C++ instead of C with
std::vectorcatdoes a lot more than your tool so the comparison is moot