pentapiper/pupdns
1
0
Fork 0
Code Issues Pull requests 1 Projects Releases Packages Wiki Activity Actions

packet-parsing #1

Open
pentapiper wants to merge 5 commits from packet-parsing into main
pull from: packet-parsing
merge into: pentapiper:main
Conversation 6 Commits 5 Files changed 8 +53 -4
5 changed files with 53 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 68400a25b7 - Show all commits

7
include/dns_packet_buffer.h
View file

@ -1,13 +1,16 @@
#pragma once #pragma once
#include <stddef.h> #include <stddef.h>
#define PACKET_BUFFER_SIZE 512 #define PB_SIZE 512
#define PB_OUT_OF_BOUNDS 9999;
struct DNSPacketBuffer { struct DNSPacketBuffer {
affine commented 2025-07-18 18:13:14 -07:00
Copy link

Im used to seeing underscores instead of camelcase in C projects, but thats just an opinion.

Im used to seeing underscores instead of camelcase in C projects, but thats just an opinion.
char buf[PACKET_BUFFER_SIZE]; char buf[PB_SIZE];
size_t pos; size_t pos;
}; };
struct DNSPacketBuffer *new_dns_packet_buffer(); struct DNSPacketBuffer *new_dns_packet_buffer();
affine commented 2025-07-18 18:13:47 -07:00
Copy link

I think both of the structs in this file should be typedef'ed so that you dont need to write 'struct' everywhere.

I think both of the structs in this file should be typedef'ed so that you dont need to write 'struct' everywhere.
struct DNSPacketHeader *dns_header(struct DNSPacketBuffer); struct DNSPacketHeader *dns_header(struct DNSPacketBuffer);
char pb_read_byte(struct DNSPacketBuffer *);

5
include/dns_request.h
View file

@ -1,7 +1,10 @@
#pragma once #pragma once
#include "dns_packets.h" #include "dns_packets.h"
#include "dns_packet_buffer.h"
struct DNSRequest { struct DNSRequest {
struct DNSPacketHeader header; struct DNSPacketHeader *header;
}; };
struct DNSRequest *new_dns_request(struct DNSPacketBuffer *pb);
affine commented 2025-07-18 18:03:18 -07:00
Copy link

recommend typedef'ing this so that you can use DNSRequest as a first class type name and not need to add "struct" everywhere.

recommend typedef'ing this so that you can use DNSRequest as a first class type name and not need to add "struct" everywhere.

2
include/udp_server.h
View file

@ -1,5 +1,5 @@
#pragma once #pragma once
#define UDP_PORT 53 #define UDP_PORT 1053
affine commented 2025-07-18 18:05:33 -07:00
Copy link

I think there is a way to leave this at 53 and then use compiler flags to redefine it. Might be a good idea.

Or you could use an IFNDEF clause to have one port for debug builds and one port for release builds based on whether or not cmake is compiling this with debug info.

I think there is a way to leave this at 53 and then use compiler flags to redefine it. Might be a good idea. Or you could use an IFNDEF clause to have one port for debug builds and one port for release builds based on whether or not cmake is compiling this with debug info.
int start_server(); int start_server();

32
src/dns_packet_buffer.c
View file

@ -1,4 +1,5 @@
#include "dns_packet_buffer.h" #include "dns_packet_buffer.h"
#include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
struct DNSPacketBuffer *new_dns_packet_buffer() struct DNSPacketBuffer *new_dns_packet_buffer()
affine commented 2025-07-18 18:12:42 -07:00
Copy link

What is the point of this getter? I could collapse it down to one line:

struct DNSPacketBuffer *buf = (struct DNSPacketBuffer *) malloc(sizeof(struct DNSPacketBuffer));

If you implement a typedef for this that one liner gets even more concise:

DNSPacketBuffer *buf = (DNSPacketBuffer *) malloc(sizeof(DNSPacketBuffer));

Also, I think it is a really bad idea to have malloc called in scattered functions like this. One honest recommendation I can make is to make sure that the point in code where you need memory should be the same place that you both allocate and free it.

In essence: as much as possible every function that calls malloc should also call free.

I have also seen larger applications keep pools associated with lifetimes. NGINX and NGINX Unit both keep a memory pool to allocate out of attached to a request object. Maybe a simpler implementation could be to create a request_resources struct that tracks shared state through the request processing pipeline. You could store pointers to all your malloc()'ed stuff there and then at the very end implement logic that calls free() on everything attached to the request.

What is the point of this getter? I could collapse it down to one line: ```c struct DNSPacketBuffer *buf = (struct DNSPacketBuffer *) malloc(sizeof(struct DNSPacketBuffer)); ``` If you implement a typedef for this that one liner gets even more concise: ```c DNSPacketBuffer *buf = (DNSPacketBuffer *) malloc(sizeof(DNSPacketBuffer)); ``` Also, I think it is a really bad idea to have malloc called in scattered functions like this. One honest recommendation I can make is to make sure that the point in code where you need memory should be the same place that you both allocate and free it. In essence: as much as possible every function that calls malloc should also call free. I have also seen larger applications keep pools associated with lifetimes. NGINX and NGINX Unit both keep a memory pool to allocate out of attached to a request object. Maybe a simpler implementation could be to create a request_resources struct that tracks shared state through the request processing pipeline. You could store pointers to all your malloc()'ed stuff there and then at the very end implement logic that calls free() on everything attached to the request.
@ -12,3 +13,34 @@ struct DNSPacketHeader *dns_header(struct DNSPacketBuffer)
{ {
} }
// read a single byte and move the cursor one byte forward
char pb_read_byte(struct DNSPacketBuffer *pb)
{
char b = pb->buf[pb->pos];
pb->pos++;
if (pb->pos >= PB_SIZE) {
printf(
"WARN: pb_read_byte: advanced packetbuffer cursor pos to %lu, which is greater than the max %d",
pb->pos,
PB_SIZE
);
}
return b;
}
char pb_get_byte(struct DNSPacketBuffer *pb)
{
if (pb->pos >= PB_SIZE) {
printf(
"WARN: pb_get_byte: accessing packetbuffer cursor pos %lu, which is greater than the max %d",
pb->pos,
PB_SIZE
);
}
return pb->buf[pb->pos];
}

11
src/dns_request.c
View file

@ -1,3 +1,14 @@
#include <stdlib.h>
#include "dns_request.h" #include "dns_request.h"
struct DNSRequest *new_dns_request(struct DNSPacketBuffer *pb)
{
struct DNSRequest *req;
req = malloc(sizeof(struct DNSRequest));
req->header = (struct DNSPacketHeader *) pb->buf;
return req;
}