On stream review
braddotcoffee opened this issue · 2 comments
braddotcoffee commented
General
- SICK Graphics and I'm super impressed with the project
Security (strcmp vs strncmp)
- Technically you can run into buffer overflow/overrun issues with the
str
functions - I'd recommend using their
strn
counterparts where you're required to specify a maximum size - What is Buffer Overflow
- strncmp manpage
Security (Privilege Escalation)
- Code reference
- Technically speaking applications that have access to editing environment variables do not necessarily have sudo access
- As a result, you run the risk of privilege escalation if you have an app installed that modifies
EDITOR
to be some bash commands that it wants to run and you then runmodifyRepo
- Generally would be safer to not rely on the environment variable here because they are modifiable by other applications
- Instead take a model similar to what git does with its editor where you configure it explicitly from the command line
Constants
- Might be worth trying to define your constants as
const <type> <name>
rather than macros
Early Return Design Pattern
- A lot of the time you have functions that are in the form of
if (cond) {
// main logic
} else {
// error handling
}
- Try out writing these as:
if (!cond) {
// error handling
return
}
// main logic
- It tends to be more readable and you might end up preferring it!
Comments
- We simply must comment less
- Try your hand at writing an entire file with zero comments
- Rely on variable names / function names to convey the context that you're trying to convey via comments
- This is just an exercise - we do not have to be this dogmatic long term. As a learning exercise I think it could help you grow as an engineer
statulr commented
Starting now
statulr commented
Patched I really appreciate all the help man and all the shoutouts! You made my day! Thank you for helping me maintain aurc by keeping it up to par with these suggestions! I took all of them into account :D @braddotcoffee