No need for 0x%p, the %p formatting already puts a 0x on. Also, no need for the (void*) cast of bp to print it.

Probably should change this free/malloc sequence to a reallocf() call. Then we can optimize libstand realloc() to do nothing if the existing buffer is big enough already (as a separate piece of work from these changes).

The code in libstand mostly follows the style(9) rule of not initializing a variable in the definition of it. (I hate that rule, it's one of the dumbest, but even so I tend to follow it when the nearby code is already doing so.)

uint32_t (since it's changed below for this var). same in the decode function below

Setting all these local variables to 0 before returning is a bit odd. Likewise on other error-exit paths. Was this related to a "goto done" type scheme that got removed or didn't get finished?

Does pkt leak away on this exit path?

I'm not sure this +2 to align ip headers is a good idea for all arches. We had problems with the uboot loader and uboot's IO API routines complaining about doing IO that wasn't on cacheline boundaries. We ended up tweaking libstand's malloc to make sure that all allocations were aligned to cacheline size to fix the problem (see libstand/zalloc_defs.h).

I suspect the platforms that use OF may have their own alignment requirements, and the same may be true of the efi code when uboot starts supporting efi (well, when we start using that support which could be any time now).

yep, and we have %zd for ssize_t.

except the libstand does not have reallocf() so this idea has to wait:)

Those are static variables, as the readdir there is implemented in an way to fetch the data, then process one dirent at the time - so we need to be careful not to step on the stale data. But yes, we can rework the cleanup there.

It is stored in the static variable, so the next calls will take care of it. Of course such readdir implementation itself is a bit bad, because it is sort of assuming the entire list will get processed, but I think it would deserve separate issue.

I do agree; I did use the simplest approach in this cycle, in hope that setting the alignment for ethernet header is ok for most platforms, this hope is actually based on the similar approach used by grub2.

However, I think we need better solution, mbuf like structure to provide the buffer space itself and with pointer(s) to actual data, so the custom alignment could be applied per platform. However, the focus of this work is about reworking the read buffer build to provide the read buffer where the content is created, and to avoid guessing. It may be reasonable to implement the buffer "metadata" as part of this update, or maybe as followup. I am not entirely sure which way is better.

I suspect this should be readether(d, &ptr, ...);

Coverity reports this as a NULL pointer dereference (*pkt = ptr; in readether()), CID 1374944, and it does look like a real bug to me.

yes, missing & operator here. review posted;)