I have committed the cleanup to firmware(9) discussed earlier on this list, see the commit log below. There is a remaining issue on which i would like some advice. From the commit log: Note also that there is a subtle issue with the implementation of firmware_register(): currently, as in the previous version, we just store a reference to the 'imagename' argument, but we should rather copy it because there is no guarantee that this is a static string. Now, what do you think is the best way to handle this ? The existing code tries not to allocate memory on a firmware_register(), not sure if it is on purpose (e.g. to avoid sleeping) or for other reasons. To preserve this, we should use static buffers for the image names, and pick a reasonable size for them (128 ? 256 ? 1025 ?). If on the other hand, we can afford a malloc in firmware_register(), i'd move the whole registry to a linked list, to avoid the hard limit of 30 slots in the firmware table. suggestions ? cheers luigi > luigi 2007-02-15 17:21:31 UTC > > FreeBSD src repository > > Modified files: > sys/arm/xscale/ixp425 ixp425_npe.c > sys/dev/ipw if_ipw.c if_ipwvar.h > sys/dev/isp isp_freebsd.h > sys/dev/iwi if_iwi.c if_iwivar.h > sys/dev/mxge if_mxge.c > sys/kern subr_firmware.c > sys/sys firmware.h > sys/tools fw_stub.awk > Log: > Cleanup and document the implementation of firmware(9) based on > a version that i posted earlier on the -current mailing list, > and subsequent feedback received. > > The core of the change is just in sys/firmware.h and kern/subr_firmware.c, > while other files are just adaptation of the clients to the ABI change > (const-ification of some parameters and hiding of internal info, > so this is fully compatible at the binary level). > > In detail: > - reduce the amount of information exported to clients in struct firmware, > and constify the pointer; > > - internally, document and simplify the implementation of the various > functions, and make sure error conditions are dealt with properly. > > The diffs are large, but the code is really straightforward now (i hope). > > Note also that there is a subtle issue with the implementation of > firmware_register(): currently, as in the previous version, we just > store a reference to the 'imagename' argument, but we should rather > copy it because there is no guarantee that this is a static string. > I realised this while testing this code, but i prefer to fix it in > a later commit -- there is no regression with respect to the past. > > Note, too, that the version in RELENG_6 has various bugs including > missing locks around the module release calls, mishandling of modules > loaded by /boot/loader, and so on, so an MFC is absolutely necessary > there. I was just postponing it until this cleanup to avoid doing > things twice. > > MFC after: 1 week > > Revision Changes Path > 1.2 +1 -1 src/sys/arm/xscale/ixp425/ixp425_npe.c > 1.23 +1 -1 src/sys/dev/ipw/if_ipw.c > 1.6 +1 -1 src/sys/dev/ipw/if_ipwvar.h > 1.99 +1 -1 src/sys/dev/isp/isp_freebsd.h > 1.46 +2 -2 src/sys/dev/iwi/if_iwi.c > 1.12 +1 -1 src/sys/dev/iwi/if_iwivar.h > 1.21 +1 -1 src/sys/dev/mxge/if_mxge.c > 1.9 +272 -153 src/sys/kern/subr_firmware.c > 1.4 +15 -16 src/sys/sys/firmware.h > 1.4 +1 -1 src/sys/tools/fw_stub.awkReceived on Thu Feb 15 2007 - 16:43:44 UTC
This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:39:05 UTC