On 11 Aug 2015 13:11, Mikulas Patocka wrote:
> --- /dev/null
> +++ strace/dm.c
> @@ -0,0 +1,354 @@
> +#include "defs.h"
all files should have a comment block at the top. see mtd.c as an example.
> +{
> + switch (code) {
> + case DM_REMOVE_ALL:
case statements should be at the same indent level as the switch. see mtd.c.
> + size_t offset = ioc->data_start;
data_start is a __u32, so this should be uint32_t instead of size_t
> + tprintf(", {sector_start=%llu, length=%llu",
> + (unsigned long long)s->sector_start,
> + (unsigned long long)s->length);
sector_start/length are uint64_t, not unsigned long long. please use PRIu64
defines from inttypes.h instead of random casts. you can find plenty of
examples in the tree already.
> + if (!entering(tcp))
> + tprintf(", status=%d", (int)s->status);
status is int32_t, not int, so please use PRId32 instead of int casts.
> +static void
> +dm_decode_dm_target_deps(const struct dm_ioctl *ioc, const char *extra,
> + size_t extra_size)
> +{
> + size_t offset = ioc->data_start;
uint32_t, not size_t
> + if (offset + offsetof(struct dm_target_deps, dev) >= offset &&
> + offset + offsetof(struct dm_target_deps, dev) <= extra_size) {
> + uint32_t i;
> + size_t space = (extra_size - (offset +
> + offsetof(struct dm_target_deps, dev))) / sizeof(__u64);
> + const struct dm_target_deps *s =
> + (const struct dm_target_deps *)(extra + offset);
> + if (s->count > space)
> + goto misplaced;
count is __u32, so space should probably be uint32_t instead of size_t
> +misplaced:
it's not standard in strace, but personal preference is to put a
single space before labels to help out with diff context
> + size_t offset = ioc->data_start;
uint32_t
> +misplaced:
add a space
looks like many of these comments apply to the rest of this file,
so i'll refrain from repeating myself
-mike
Attachment:
signature.asc
Description: Digital signature
-- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
