fts: Fix GCC compile error

[olce]

Summary

GCC does not support the blocks extension, contrary to clang with which
it is automatically enabled by our infrastructure (see
'lib/libc/gen/Makefile.inc') when compiling 'fts.c'. The alternate code
(blocks extension not supported/enabled) tried to dereference a 'void *'
pointer (field 'fts_compar_b' of 'FTS') to access field 'isa' of the
block mocked by 'block_abi.h'. Fix this by casting the pointer to the
block type.

Fixes: f0ac5e919f3f ("fts: Add blocks support.")
Sponsored by: The FreeBSD Foundation

[jhb]

In block_abi.h there is a GET_BLOCK_FUNCTION macro, and I sort of wonder if a GET_BLOCK_ISA wouldn't be useful instead (maybe if we ever have to fix this sort of thing in the future it will be useful to factor out such a macro)? OTOH, I can't see any other reason for the typedef DECLARE_BLOCK in fts.c except to handle this case, so I think this patch is fine as-is.

[theraven]

lib/libc/gen/fts.c

355–356

I missed this in the original review (sorry), but I think this check is not correct.

On line 254, we unconditionally return with an error if we haven't linked a blocks runtime. This means that we never need to do the comparison against _NSConcreteGlobalBlock. There are three cases at the start:

• A blocks runtime is present, in which case it's unconditionally fine to call _Block_copy and _Block_release (with or without their macro wrappers).
• A blocks runtime is not present, but the block captures only globals (its isa pointer points to _NSConcreteGlobalBlock), in which case it's fine to skip the blocks calls if they exist, or not if they don't.
• A blocks runtime is not present and fts_open_b is called with a stack block (it cannot be a malloc block, these are created only by promoting stack blocks to the heap as a result of _Block_copy). In this case, we must call _Block_copy or fail on creation.

This reduces to two cases by this point:

• It was a global block, in which case we can skip the _Block_release call.
• It is a malloc block, in which case we must call _Block_release or we will leak memory.

All of these are *unrelated* to whether __BLOCKS__ is defined.

With this in mind:

The check on line 254 should be checking that this is a global block *or* we have linked a blocks library. It currently checks that we have linked a blocks library and, if not, fails. This means that all later checks for _NSGlobalBlock are redundant. If we have linked a blocks library, it's fine (a no-op) to call _Block_copy and _Block_release unconditionally.

If this check is fixed, then lines 290 and 353 are then wrong, because we need to perform the isa check irrespective of whether the compiler supports blocks. If a blocks runtime is not linked, we'd crash on 290 and 353 when we come to release the block.

If this is not fixed, then we can get rid of both of the isa checks because we know a blocks runtime is linked and so it's safe to call _Block_release.

[des]

lib/libc/gen/fts.c

355–356

If a blocks runtime is not linked, we'd crash on 290 and 353

If a blocks runtime is not linked, fts_open_b() immediately fails with ENOSYS and we never get to 290 or 353.