Contribute/BiteSizedTasks: Difference between revisions
(hexdump has been done) |
(Added another small task to remove unused functions and variables) |
||
Line 61: | Line 61: | ||
* Once the above change is done, remove the "Error **" argument from functions named *_unrealize in hw/ | * Once the above change is done, remove the "Error **" argument from functions named *_unrealize in hw/ | ||
* Remove bdrv_aio_multiwrite() since virtio-blk no longer uses it and the only remaining caller, qemu-io, is for testing only | * Remove bdrv_aio_multiwrite() since virtio-blk no longer uses it and the only remaining caller, qemu-io, is for testing only | ||
* Use the technique described at https://blog.flameeyes.eu/2008/01/today-how-to-identify-unused-exported-functions-and-variables to identify unused global functions and variables, try to decide whether it's a good idea to remove the detected code fragments and send (single) patches to remove them if applicable. | |||
== Tracing == | == Tracing == |
Revision as of 08:24, 1 April 2016
Note: Before starting on one of these tasks, it would be wise to double-check the list archives to ensure no one else has recently submitted similar cleanups for the same task.
Note: Some of these tasks are described very briefly and somewhat cryptically. If you're not sure what a task involves, then feel free to ask for clarification on the qemu-devel mailing list.
API conversion
- Look for uses of malloc, and convert them to either g_malloc, g_new (more rarely g_try_malloc or g_try_new if a lot of memory is being allocated). Likewise, convert calloc to either g_new0 or g_try_new0. Drop return value checks unless using g_try_new/g_try_new0.
- Associate external libraries with the object files that actually use them
For all "QEMUTimer*" variables that are initialized with timer_new, change them to "QEMUTimer" and initialize them with timer_init. Drop any timer_free calls (there aren't many, so this patch would fix small memory leaks too).(not quite bite-sized).- Replace calls to functions named cpu_physical_memory_* with address_space_*.
- Change net/socket.c to use the functions in include/qemu/sockets.h instead of parse_host_port/bind/connect/listen.
- Change QemuMutex and QemuCond to CompatGMutex and CompatGCond (these are the same as GMutex and GCond, just with a different type). With this change, qemu_mutex_init/qemu_cond_init becomes optional for global variables.
- Change get_ticks_per_sec() calls to just use the NANOSECONDS_PER_SECOND constant.
Remove macros IO_READ_PROTO and IO_WRITE_PROTO.- Replace calls to object_child_foreach() with object_child_foreach_recursive() when applicable.
- Change basename and dirname to g_path_get_basename() and g_path_get_dirname() respectively.
- Devices which create MemoryRegions with the old_mmio accessors should be converted to new style mmio accessors.
- The CodeTransitions page tracks ongoing internal QEMU API transitions. Most are not trivial but it's a good source of ideas.
Header cleanups
avoid including include/exec/exec-all.h from other headers.(now owned by Paolo Bonzini)- avoid including files from include/exec/cpu-common.h.
- move declarations out of qemu-common.h for functions declared in utils/ files: e.g. include/qemu/path.h for utils/path.c, include/qemu/id.h for utils/id.c, etc.
- move inline functions out of qemu-common.h and into new files (e.g. include/qemu/bcd.h).
Device models
- Include SDState by value instead of allocating it in sd_init (hw/sd/).
- Look for invocations of qemu_system_reset_request() in hw/. Whenever they correspond to some kind of watchdog that has triggered, change to watchdog_perform_action().
Error checking
- Add checks for NULL return value to uses of qemu_find_file.
- Add checks for negative return value to uses of load_image_targphys, get_image_size, event_notifier_init, msix_init.
- Use qemu_strtol/qemu_strtoul/qemu_strtoll/qemu_strtoull more.
Device lifecycle
- IDE uses qemu_add_vm_change_state_handler() without a corresponding qemu_del_vm_change_state_handler(). This means hot unplugging an AHCI PCI adapter results in a dangling change state handler and could lead to a crash.
Large frames
- The following functions have very large stack frames (as obtained with -Wstack-usage=16383), mostly due to huge arrays. Make the stack array smaller and allocate on the heap in the rare case that the data does not fit in the small array:
hw/dma/xilinx_axidma.c stream_process_mem2s 16480 bytes hw/net/virtio-net.c virtio_net_receive 16544 bytes net/tap.c net_init_tap 16752 bytes block/vmdk.c vmdk_write_cid.isra.8 20544 bytes hw/display/vmware_vga.c vmsvga_fifo_run 20688 bytes hw/virtio/virtio.c virtqueue_pop 24768 bytes hw/net/virtio-net.c virtio_net_flush_tx 32928 bytes hw/virtio/virtio.c qemu_get_virtqueue_element 49216 bytes hw/virtio/virtio.c qemu_put_virtqueue_element 49216 bytes hw/net/opencores_eth.c open_eth_start_xmit 65664 bytes hw/net/spapr_llan.c h_send_logical_lan ~64Kbytes (uses alloca) hw/arm/nseries.c n8x0_init 65728 bytes net/net.c nc_sendv_compat 69680 bytes net/socket.c net_socket_send 69712 bytes
- For a larger list of functions with stack sizes of 1K or bigger, see http://article.gmane.org/gmane.comp.emulators.qemu/314061
Dead code removal
- hw/display contains files named *_template.h. These are included many times with different values of the DEPTH macro. However, only the DEPTH == 32 case is used. Remove support for DEPTH != 32 in the template headers and in the file that include them.
- Look for functions that are named *_exit or *_exitfn in hw/ and that return int. They should all return zero. Make them return void, and remove the checks for the callers.
- Once the above change is done, remove the "Error **" argument from functions named *_unrealize in hw/
- Remove bdrv_aio_multiwrite() since virtio-blk no longer uses it and the only remaining caller, qemu-io, is for testing only
- Use the technique described at https://blog.flameeyes.eu/2008/01/today-how-to-identify-unused-exported-functions-and-variables to identify unused global functions and variables, try to decide whether it's a good idea to remove the detected code fragments and send (single) patches to remove them if applicable.
Tracing
- Add tracepoints. All functions that are named something_helper, and all functions mentioned in MemoryRegionOps are good candidates.
Bitrot prevention
- Files with conditional debug statements should ensure that the printf is always compiled, and merely hidden behind
if (0)
when not debugging, rather than#ifdef
'd out. This prevents bitrot of the format string of the debug statement. See commit c691320faa6 for an example. Or, go one step further and convert the debug statements to tracepoints.
Misc
Improve the qemu_hexdump() function in util/hexdump.c to also print out an ASCII dump of the buffer (just like the hex_dump() function in net/net.c is doing it).Remove the duplicated hex_dump() function in net/net.c, and use qemu_hexdump() (from util/hexdump.c) instead.