added sntp, dns and updated timer and net_proc#26
added sntp, dns and updated timer and net_proc#26differrari merged 7 commits intodifferrari:mainfrom
Conversation
|
If I could make a small gentle constructive criticism of this PR, it would be useful if you broke your changes down in to smaller meaningful commits with informative commit messages. It becomes difficult to review larger changes like this, but viewing the branch history as a "story" about the changes being made and why makes it more digestible. This isn't to critique your actual changes, there is no doubt they improve the code, but just a random comment from a random coder who maintained random FOSS repos in the past. |
|
Heya, thanks for the tip. I know it's not great to do a single huge commit (Ive been told that many times already, lmao), but I've mostly worked locally and havent used git much, so every time I try to do something with it i usually mess things up. Anyway, Ill try to get better at it, even if git drives me a bit crazy |
| static port_entry_t g_port_table[PROTO_COUNT][MAX_PORTS];//tab proto/port | ||
|
|
||
| static inline bool port_valid(uint16_t p) { | ||
| static inline bool port_valid(uint32_t p) { |
There was a problem hiding this comment.
Changing p parameter to uint32_t doesn't add any value here if all of the arguments ever passed to it are converted/truncated to 16-bits. If every value passed to this function is already between 0 and MAX_PORTS then at worst it wastes some CPU cycles for nothing and at best it gets optimized-out.
I removed it again in the update compiler warnings clean up branch. It was disused in the network changes which landed in master branch anyway, so kind of moot.
| memcpy(pkt->payload, d->payload, 56); | ||
| else | ||
| memset(pkt->payload, 0, 56); | ||
| memset(pkt->payload, 0, sizeof(pkt->payload)); |
There was a problem hiding this comment.
This memset call is unnecessary since the following line completely overwrites the entire pkt->payload anyway. It would probably get optimized-out if the project were built with optimizations, but in it's current state, it likely just wastes some CPU cycles.
| #define TCP_RETRY_TIMEOUT_MS 1000 | ||
|
|
||
| static int find_flow(uint16_t local_port, uint32_t remote_ip, uint16_t remote_port); | ||
| int find_flow(uint16_t local_port, uint32_t remote_ip, uint16_t remote_port); |
There was a problem hiding this comment.
I don't think this is the correct fix for the compiler warning it generates. The find_flow function is only ever used inside the tcp.c file, so it should remain static there, removed from the header here since it's not used in any other files, and then in tcp.c it should just be moved up in the file so that it's defined before use.
3 participants
added sntp client and sntp daemon
added dns resolver and dns daemon
updated timer to support sntp real/unix time alongside the monotonic clock
added DateTime type + helper and manual timezone offset support
cleaned up net_proc; better logs and moved the PID setter inside the daemons
renamed dhcp/arp getter