Contribute/BiteSizedTasks

From QEMU
Revision as of 07:45, 18 November 2021 by Huth (talk | contribs) (Patch submission information has been moved to the docs in the git repo)

This list is in the process of being re-evaluated and migrated to the QEMU issue tracker using the Bite Sized issue label.

For QEMU maintainers: Please add your new suggestions to the issue tracker instead, and add the Bite Sized label. Please migrate any existing still relevant ideas to the issue tracker and remove them from this page afterward.

For new contributors: Before starting on one of the tasks on this page, you should check the mailing list archives to ensure no one else has recently submitted similar cleanups for the same task. If nobody has, and the task appears valid, please reach out on the QEMU development mailing list and let us know you'd like to start working on the issue. Please CC: John Snow <jsnow AT redhat.com> when doing so, they'll help get you assigned a GitLab issue and find the right points of contact.

For tasks on the issue tracker or on this page, patches are still handled by submitting patches to the mailing list and not via Gitlab merge requests. Before submitting a patch to the mailing list, please make sure that you've read and understood the patch submission page.

Gitlab "Bite Sized" Issues

Issues tagged "Bite Sized" on the issue tracker may be good candidates for first contributions. If you're interested in solving one of these problems, please register for a Gitlab account and participate in the discussion on the issue so it will be evident to others that you are engaged in researching and fixing the issue.

The issues may sometimes appear to be cryptic or vague. Don't hesitate to ask for more information if the task is not clear. If there is no obvious point of contact from the issue page itself, please reach out for help on the qemu development mailing list.

API conversion

  • Replace manual qemu_mutex_lock() and qemu_mutex_unlock() calls with QEMU_LOCK_GUARD() where it makes the code easier to read. See ToDo/LockGuards for details.
  • Look for uses of malloc, and convert them to g_new or similar (see the coding style guide for more details on allocation interface usage). Likewise, convert calloc to g_new0 and friends. Drop return value checks unless using g_try_new/g_try_new0. (This needs to be done for one data structure at a time, because an allocation with malloc must be freed with free but an allocation with g_malloc must be freed with g_free. So you need to do a bit of analysis of the code to see where an allocation may be later freed.) Please ignore the mallocs in libdecnumber and tests/tcg/.
  • Replace calls to functions named cpu_physical_memory_* with address_space_*.
  • Replace calls to object_child_foreach() with object_child_foreach_recursive() when applicable: nvdimm_device_list, nmi_monitor_handle, find_sysbus_device, pc_dimm_slot2bitmap, build_dimm_list.
  • The ToDo/CodeTransitions page tracks ongoing internal QEMU API transitions. Most are not trivial but it's a good source of ideas, and some items should be doable for newcomers, too.
  • Introduce a better alternative to strncpy. QEMU uses strncpy whenever the destination needs to be zeroed entirely, for example to avoid data leaks. Add a new function qemu_strncpy that asserts that the destination buffer is big enough to fit strlen(source)+1 bytes. Also add a new function qemu_strncpy_nonul that asserts that the destination buffer is big enough to fit strlen(source) bytes (thus not guaranteeing NUL termination). Replace uses of strncpy throughout the QEMU sources.

Code Modernization

  • Convert routines with multiple goto exit-paths to use g_autoptr/g_autofree to handle clean-up and allow direct returns
  • Replace common idioms like if (s->len > 0) { g_string_append(s, ", "); } g_string_append(s, "foo") with common helper function

Device models

  • 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().
  • Add support for attribute((bitwise)), tag structure fields and use that for static checking of structure endian-ness accesses
  • Implement the "Backend program conventions" from vhost-user.rst, including the standard command-line options and vhost-user.json for contrib/vhost-user-blk/ and contrib/vhost-user-scsi/. See contrib/vhost-user-input/ for an example.

Error checking

  • Use qemu_strtol/qemu_strtoul/qemu_strtoll/qemu_strtoull more. (contact: eblake)

Compiler-driven cleanups

  • Use of -Wshadow while compiling can prevent legitimate bugs, but can't be enabled until we first clean up the code base to avoid shadowed declarations.
  • Use of -Wvla and -Wframe-larger-than=4096 can prevent the use of potential security flaws caused by stack overflows made possible with variable-length arrays or other over-large stack allocations.
  • 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
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/arm/nseries.c                     n8x0_init                        65728 bytes
net/net.c                            nc_sendv_compat                  69680 bytes
net/socket.c                         net_socket_send                  69712 bytes

Tracing

  • Add tracepoints. All functions that are named something_helper, and all functions mentioned in MemoryRegionOps are good candidates.
  • Convert DPRINTF() calls to tracepoints.

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 this (commit c691320faa6) for an example. Or, go one step further and convert the debug statements to tracepoints. (contact: eblake)

Functions that should be static

  • For example: vga_sync_dirty_bitmap, WC_CONFIG_STRING, WC_FULL_CONFIG_STRING, WC_MODEL_STRING, timer_put, vfio_region_ops, vfio_reset_handler, cpu_gen_init, monitor_read_bdrv_key_start, pdu_marshal, pdu_unmarshal, qcrypto_hmac_alg_map (crypto/hmap-nettle.c), spapr_tce_set_need_vfio, virtqueue_map, vnc_disconnect_finish, vnc_display_init, gicv3_full_update_noirqset, cpu_disable_ticks, cpu_enable_ticks, qemu_thread_exit (actually unused in thread-posix.c), qemu_egl_init_dpy_mesa, monitor_defs (3 occurrences), apic_deliver_irq, pcie_host_mmcfg_map, pcie_host_mmcfg_unmap
  • tracetool-generated arrays (e.g. hw_mem_trace_events in the generated file hw/mem/trace.c) should be static.
  • The property types defined in hw/core/qdev-properties-system.c should be moved to other directories (e.g. hw/net for network-related property types). After doing this, some functions probably could become static, too.

Contact persons

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, or the contact person for the task (if the task has been marked with a contact):

  • eblake: Eric Blake (mail: eblake AT redhat.com -- IRC: eblake)
  • jsnow: John Snow (mail: jsnow AT redhat.com -- IRC: jsnow)