Opened 2 years ago

Closed 2 months ago

#64909 closed defect (fixed)

libaacs for PowerPC: how to rewrite __block code in standard C?

Reported by: barracuda156 Owned by: i0ntempest
Priority: Normal Milestone:
Component: ports Version: 2.7.2
Keywords: powerpc, leopard, snowleopard Cc:
Port: libaacs

Description (last modified by ryandesign (Ryan Carsten Schmidt))

Could anyone advise me if this can be rewritten in standard C? gcc does not process this __block code correctly:

void device_close(MMCDEV **pp)
{
    __block int rc = 0;
    if (pp && *pp) {
        MMCDEV *mmc = *pp;

        /* When the exclusive access to the drive is released,
         * the OS will see the device like a "new" device and
         * try to mount it. Therefore we can't just mount the
         * disk we previously got immediately here as it would
         * fail with kDAReturnBadArgument as the disk is not
         * available yet.
         * Trying to mount the disk after it appears in peek
         * does not work either as the disk is not yet ready
         * or in the process of being mounted by the OS so
         * that would return an kDAReturnBusy error.
         * The only way that seems to reliably work is to use
         * a mount approval callback. When the OS tries to
         * mount the disk, the mount approval callback is
         * called and we can reject mounting and then proceed
         * to mount the disk ourselves.
         * Claiming exclusive access using DADiskClaim in order
         * to prevent the OS form mounting the disk does not work
         * either!
         */

        if (mmc->taskInterface) {
            (*mmc->taskInterface)->ReleaseExclusiveAccess(mmc->taskInterface);
            (*mmc->taskInterface)->Release(mmc->taskInterface);
            mmc->taskInterface = NULL;
        }

        if (mmc->mmcInterface) {
            (*mmc->mmcInterface)->Release(mmc->mmcInterface);
            mmc->mmcInterface = NULL;
        }

        if (mmc->plugInInterface) {
            IODestroyPlugInInterface(mmc->plugInInterface);
        }

        if (!mmc->sync_sem) {
            /* open failed before iokit_da_init() */
            X_FREE(*pp);
            return;
        }

        /* Wait for disc to re-appear for 20 seconds.
         * This timeout was figured out by experimentation with
         * a USB BD drive which sometimes can take really long to
         * be in a mountable state again.
         * For internal drives this is probably much faster
         * so the long timeout shouldn't do much harm for thse
         * cases.
         */
        dispatch_time_t timeout = dispatch_time(DISPATCH_TIME_NOW, 20 * 1E+9);
        dispatch_semaphore_wait(mmc->sync_sem, timeout);

        /* It is crucial that this is done on the event handling queue
         * else callbacks could be received while this code runs.
         */
        dispatch_sync(mmc->background_queue, ^{
            if (disk_appeared != mmc->disk_state) {
                BD_DEBUG(DBG_MMC | DBG_CRIT, "Timeout waiting for the disc to appear again!\n");
                iokit_da_destroy(mmc);
                rc = -1;
                return;
            }
            rc = 0;
        });

        if (rc == 0) {
            /* Disk appeared successfully, mount it.
             * Return value is ignored as logging of success or
             * error takes place in the callback already and there
             * is nothing we can do really if mounting fails.
             */
            (void) iokit_mount(mmc);
            iokit_da_destroy(mmc);
        }
        X_FREE(*pp);
    }
}

This is from src/file/mmc_device_darwin.c.

Change History (3)

comment:1 Changed 2 years ago by ryandesign (Ryan Carsten Schmidt)

Description: modified (diff)
Owner: set to i0ntempest
Status: newassigned
Type: requestdefect

FYI:

Here's what blocks are: https://en.wikipedia.org/wiki/Blocks_(C_language_extension)

Here's the ticket about gcc not supporting them yet: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78352

Probably the only sane path forward for this is for gcc to implement support for blocks. Then this port can use gcc to build on PowerPC. It's probably unreasonable to expect upstream to abandon the useful blocks feature in their code for the benefit of ancient systems, and it's probably unreasonable for MacPorts to develop and carry forward forever a large patch for this.

comment:2 in reply to:  1 Changed 2 months ago by barracuda156

Replying to ryandesign:

FYI:

Here's what blocks are: https://en.wikipedia.org/wiki/Blocks_(C_language_extension)

Here's the ticket about gcc not supporting them yet: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78352

Probably the only sane path forward for this is for gcc to implement support for blocks. Then this port can use gcc to build on PowerPC. It's probably unreasonable to expect upstream to abandon the useful blocks feature in their code for the benefit of ancient systems, and it's probably unreasonable for MacPorts to develop and carry forward forever a large patch for this.

Thank you, Ryan, and I apologize for somehow missing your reply earlier.

While gcc seems to intend to implement the support for blocks (at least according to Iain), I think for the time-being it is better to provide a bit earlier version for PowerPC systems. 0.9.0 builds fine.

comment:3 Changed 2 months ago by barracuda156

Resolution: fixed
Status: assignedclosed

In 98365711630b55bbafc28c9b7f51a7f6e95e1e46/macports-ports (master):

libaacs: provide fallback for ppc where no clang works

Closes: #64909

Note: See TracTickets for help on using tickets.