On Tue, Aug 23, 2016 at 01:56:03PM -0400, robert.foss@xxxxxxxxxxxxx wrote:
> From: Robert Foss <robert.foss@xxxxxxxxxxxxx>
>
> Base functions to help testing the Sync File Framework (explicit fencing
> mechanism ported from Android).
> These functions allow you to create, use and destroy timelines and fences.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
> Signed-off-by: Robert Foss <robert.foss@xxxxxxxxxxxxx>
> ---
> lib/Makefile.sources | 2 +
> lib/sw_sync.c | 238 +++++++++++++++++++++++++++++++++++++++++++++++++++
> lib/sw_sync.h | 49 +++++++++++
> 3 files changed, 289 insertions(+)
> create mode 100644 lib/sw_sync.c
> create mode 100644 lib/sw_sync.h
>
[snip]
> +int sw_sync_fd_is_valid(int fd)
> +{
> + int status;
> +
> + if (fd == -1)
`-1` seems too specific. While open() will return -1 on error, any
negative fd is invalid, so I'd test for `<0` here instead.
> + return 0;
> +
> + status = fcntl(fd, F_GETFD, 0);
> + return status >= 0;
> +}
> +
> +static
> +void sw_sync_fd_close(int fd)
> +{
> + if (fd == -1)
> + return;
> +
> + if (fcntl(fd, F_GETFD, 0) < 0)
> + return;
Why not replace these two tests with a simple
if (sw_sync_fd_is_valid(fd))
> +
> + close(fd);
> +}
> +
> +int sw_sync_timeline_create(void)
> +{
> + int fd = open("/dev/sw_sync", O_RDWR);
> +
> + if (!sw_sync_fd_is_valid(fd))
> + fd = open("/sys/kernel/debug/sync/sw_sync", O_RDWR);
I tend to prefer for hard-coded paths to be in a #define at the top, but
I don't know what the policy for that is in IGT.
> +
> + igt_assert(sw_sync_fd_is_valid(fd));
> +
> + return fd;
> +}
> +
[snip]
> +static struct sync_file_info *sync_file_info(int fd)
> +{
> + struct sync_file_info *info;
> + struct sync_fence_info *fence_info;
> + int err, num_fences;
> +
> + info = malloc(sizeof(*info));
> + if (info == NULL)
> + return NULL;
> +
> + memset(info, 0, sizeof(*info));
You could replace malloc() + memset(0) with calloc(), as you're doing
a few lines below.
> + err = ioctl(fd, SYNC_IOC_FILE_INFO, info);
> + if (err < 0) {
> + free(info);
> + return NULL;
> + }
> +
> + num_fences = info->num_fences;
> +
> + if (num_fences) {
> + info->flags = 0;
> + info->num_fences = num_fences;
> +
> + fence_info = calloc(num_fences, sizeof(struct sync_fence_info));
sizeof(*fence_info)
> + if (!fence_info)
> + free(info);
> + return NULL;
Missing braces
> +
> + info->sync_fence_info = (uint64_t)(unsigned long) (fence_info);
> +
> + err = ioctl(fd, SYNC_IOC_FILE_INFO, info);
> + if (err < 0) {
> + free(fence_info);
> + free(info);
> + return NULL;
> + }
> + }
> +
> + return info;
> +}
[snip]
Cheers,
Eric
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
