Opened 10 years ago

Closed 8 years ago

#4722 closed bug (fixed)

acpi warnings during jam (gcc2 build on ubuntu 9.04)

Reported by: scottmc Owned by: czeidler
Priority: normal Milestone: R1
Component: Kits/Kernel Kit Version: R1/alpha1
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description (last modified by scottmc)

src/add-ons/kernel/bus_managers/acpi/oshaiku.c: In function `AcpiOsDeleteLock':
src/add-ons/kernel/bus_managers/acpi/oshaiku.c:666: warning: passing arg 1 of `free' discards qualifiers from pointer target type
src/add-ons/kernel/bus_managers/acpi/namespace/nsxfeval.c: In function `AcpiNsGetDeviceCallback':
src/add-ons/kernel/bus_managers/acpi/namespace/nsxfeval.c:660: warning: unused variable `Flags'
src/add-ons/kernel/bus_managers/acpi/utilities/utdelete.c: In function `AcpiUtDeleteInternalObj':
src/add-ons/kernel/bus_managers/acpi/utilities/utdelete.c:265: warning: assignment makes integer from pointer without a cast
src/add-ons/kernel/bus_managers/acpi/utilities/utdelete.c:285: warning: assignment makes integer from pointer without a cast
src/add-ons/kernel/bus_managers/acpi/utilities/utglobal.c: In function `AcpiUtInitGlobals':
src/add-ons/kernel/bus_managers/acpi/utilities/utglobal.c:869: warning: assignment makes integer from pointer without a cast
src/add-ons/kernel/bus_managers/acpi/utilities/utglobal.c:911: warning: assignment makes integer from pointer without a cast
src/add-ons/kernel/bus_managers/acpi/utilities/utlock.c: In function `AcpiUtDeleteRwLock':
src/add-ons/kernel/bus_managers/acpi/utilities/utlock.c:167: warning: assignment makes integer from pointer without a cast
src/add-ons/kernel/bus_managers/acpi/utilities/utlock.c:168: warning: assignment makes integer from pointer without a cast
src/add-ons/kernel/bus_managers/acpi/utilities/utmutex.c: In function `AcpiUtDeleteMutex':
src/add-ons/kernel/bus_managers/acpi/utilities/utmutex.c:299: warning: assignment makes integer from pointer without a cast

Attachments (1)

mutex_lock_with_timeout.diff (4.7 KB) - added by bonefish 9 years ago.
possible mutex_lock_with_timeout() implementation

Download all attachments as: .zip

Change History (17)

comment:1 Changed 10 years ago by scottmc

Description: modified (diff)

formatting fix only

comment:2 Changed 10 years ago by phoudoin

Owner: changed from axeld to phoudoin
Status: newassigned

Oh, an evil warning in oshaiku.c :-)

I'll try to fix that, after switching to gcc2.

comment:3 Changed 10 years ago by phoudoin

I most probably wont fix the "assignment makes integer from pointer without a cast" warnings, as these are due to a wrong assertion made by ACPI-CA implementers on ACPI_MUTEX type being necessary pointers, which is wrong under Haiku (it's sem_id, aka integer instead).

comment:4 Changed 10 years ago by mmlr

Any reason why it wouldn't use an actual mutex?

comment:5 Changed 10 years ago by phoudoin

Hum, let see... nope. :-)

comment:6 Changed 9 years ago by phoudoin

Owner: changed from phoudoin to czeidler
Status: assignednew

comment:7 Changed 9 years ago by czeidler

The oshaiku.c warning I fixed some time ago. I forgot to include a header... From a TODO in the haiku specific part a mutex is not used because it don't support timeouts. As far as I see this is right. Is this correct? should I use pthread_mutex instead?

comment:8 Changed 9 years ago by axeld

Those aren't available in the kernel. You might want to fall back to use semaphores, at least I think that extending mutexes would make them considerably more complex, but Ingo would know more about that.

comment:9 Changed 9 years ago by bonefish

It's not particularly complex. See the attached patch for a possible mutex_lock_with_timeout() implementation (utterly untested, though). It's mostly identical to the mutex_lock() implementation, save for the use of thread_block_with_timeout_locked() instead of thread_block_locked() and the additional cleanup needed when the timeout does indeed occur. Since in such a case it is also necessary to fix the mutex' count field (!KDEBUG), the new field ignore_unlock_count had to be added to work around a race condition with mutex_unlock().

comment:10 Changed 9 years ago by stippi

I read through the patch and I don't understand what happens in line 692 and 693.

Changed 9 years ago by bonefish

possible mutex_lock_with_timeout() implementation

comment:11 in reply to:  10 Changed 9 years ago by bonefish

Replying to stippi:

I read through the patch and I don't understand what happens in line 692 and 693.

Thanks, that was indeed incorrect. The intention was to update the last pointer of the new first object.

comment:12 Changed 9 years ago by stippi

Ah cool, so I wasn't on the wrong track. Am I assuming correctly that all list links have the "last" pointer, but only the one of the first link is maintained at all?

comment:13 in reply to:  12 Changed 9 years ago by bonefish

Replying to stippi:

Ah cool, so I wasn't on the wrong track. Am I assuming correctly that all list links have the "last" pointer, but only the one of the first link is maintained at all?

Exactly. Normally a last pointer would be in the list structure, but in this case we want to save the space in the mutex structure and use this somewhat unusual way.

comment:14 Changed 8 years ago by scottmc

Strange, this one has a possible patch attached to it, but the has a patch checkbox is crossed out, so that you can't check it. Was this fixed by hrev37680?

comment:15 Changed 8 years ago by mmadia

Has a Patch: set

comment:16 Changed 8 years ago by czeidler

Resolution: fixed
Status: newclosed

Yes the main problem is fixed. There are still some warnings but nothing important (?).

Note: See TracTickets for help on using tickets.