Skip to content

fix(contrib/mqtt): ignore unused retval#34735

Merged
guanshengliang merged 1 commit intomainfrom
fix/6779787380
Mar 11, 2026
Merged

fix(contrib/mqtt): ignore unused retval#34735
guanshengliang merged 1 commit intomainfrom
fix/6779787380

Conversation

@stephenkgu
Copy link
Contributor

Description

Issue(s)

Checklist

Please check the items in the checklist if applicable.

  • Is the user manual updated?
  • Are the test cases passed and automated?
  • Is there no significant decrease in test coverage?

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on improving code hygiene within the MQTT contributor library by explicitly marking function return values that are intentionally ignored. This change suppresses compiler warnings and clarifies the code's intent without altering its functional behavior, ensuring a cleaner build process.

Highlights

  • Compiler Warning Fixes: Addressed compiler warnings related to unused return values by wrapping relevant function calls with the UNUSED() macro across multiple C source files.
  • Code Hygiene Improvement: Enhanced code quality by explicitly indicating that certain function return values are intentionally ignored, preventing potential future issues or confusion.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • contrib/libmqtt/ttq/src/ttqMisc.c
    • Wrapped umask(old_mask) with UNUSED() to ignore its return value.
    • Wrapped getpwuid_r() with UNUSED() to ignore its return value.
    • Wrapped getgrgid_r() with UNUSED() to ignore its return value.
  • contrib/libmqtt/ttq/src/ttqNet.c
    • Wrapped multiple calls to COMPAT_CLOSE(*sock) with UNUSED().
    • Wrapped multiple calls to net__socket_close(ttq) with UNUSED().
    • Wrapped SSL_set_tlsext_host_name(ttq->ssl, host) with UNUSED().
    • Wrapped fcntl(*sock, F_GETFL, 0) and fcntl(*sock, F_SETFL, opt | O_NONBLOCK) with UNUSED().
  • contrib/libmqtt/ttq/src/ttqNet2.c
    • Wrapped net__init() with UNUSED().
    • Wrapped COMPAT_CLOSE(spare_sock) and COMPAT_CLOSE(new_sock) with UNUSED() in several instances.
    • Wrapped COMPAT_CLOSE(sock) with UNUSED() in error handling paths.
  • contrib/libmqtt/ttq/src/ttqPacket.c
    • Wrapped ttqMuxAddOut(ttq) with UNUSED().
    • Wrapped ttqMuxRemoveOut(ttq) with UNUSED().
  • contrib/libmqtt/ttq/src/ttqSendPublish.c
    • Wrapped packet__write_varint(packet, proplen) with UNUSED().
  • contrib/libmqtt/ttq/src/ttqUtil.c
    • Wrapped net__socket_close(ttq) with UNUSED() in keepalive checks.
    • Wrapped send__pingreq(ttq) with UNUSED().
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The pull request primarily introduces UNUSED() macros around various function calls in ttqMisc.c, ttqNet.c, ttqNet2.c, ttqPacket.c, ttqSendPublish.c, and ttqUtil.c to silence compiler warnings about unused return values. Review comments highlight that ignoring these return values for functions like ttqMuxAddOut(), ttqMuxRemoveOut(), packet__write_varint(), getpwuid_r(), getgrgid_r(), and net__init() could lead to issues such as stalled packets, incorrect event monitoring, malformed packets, or masked system/initialization errors. The reviewer suggests checking and handling these return values to improve robustness and prevent potential bugs.

struct passwd pw, *result;

getpwuid_r(getuid(), &pw, buf, sizeof(buf), &result);
UNUSED(getpwuid_r(getuid(), &pw, buf, sizeof(buf), &result));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While UNUSED silences the compiler warning, it's better to handle the return value of getpwuid_r. It returns an error code on failure (e.g., if the buffer is too small), which is different from not finding a user. Ignoring this error could hide underlying system issues. Consider checking the return value and logging an error if it's non-zero.

struct group grp, *result;

getgrgid_r(getgid(), &grp, buf, sizeof(buf), &result);
UNUSED(getgrgid_r(getgid(), &grp, buf, sizeof(buf), &result));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to getpwuid_r, the return value of getgrgid_r should be checked to distinguish between errors (like a too-small buffer) and a group not being found. Please consider handling the error case to make the code more robust.

int rc = net__init_ssl_ctx(ttq);
if (rc) {
net__socket_close(ttq);
UNUSED(net__socket_close(ttq));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While this change silences the warning, it's worth noting that net__socket_close can return an error. In this specific error-handling path, ignoring it is acceptable to preserve the original error code. However, in other contexts, it might be important to handle it.

void ttqNetBrokerInit(void) {
spare_sock = socket(AF_INET, SOCK_STREAM, 0);
net__init();
UNUSED(net__init());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The function net__init() can fail (e.g., if ares_library_init() fails within it), but its return value is being ignored here. This could mask initialization problems with the DNS library. It would be safer to check the return value and handle any potential failure.

@guanshengliang guanshengliang merged commit 5ce8153 into main Mar 11, 2026
12 of 13 checks passed
@guanshengliang guanshengliang deleted the fix/6779787380 branch March 11, 2026 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants