Bug 1449416 - errno used incorrectly or misleadingly in error messages
Summary: errno used incorrectly or misleadingly in error messages
Keywords:
Status: CLOSED EOL
Alias: None
Product: GlusterFS
Classification: Community
Component: geo-replication
Version: 3.10
Hardware: Unspecified
OS: Unspecified
high
unspecified
Target Milestone: ---
Assignee: bugs@gluster.org
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-05-09 22:16 UTC by nh2
Modified: 2018-06-20 18:26 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-06-20 18:26:43 UTC
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Embargoed:


Attachments (Terms of Use)

Description nh2 2017-05-09 22:16:01 UTC
I found multiple occurrences in glusterfs where code checks `errno` following a function that does not explicitly document that it sets errno.

In some cases this does not lead to bugs, as the code inside those functions really does call an inner function which sets errno and whose errno makes sense for the outer function.

In other cases though, the errno can come from inner functions that have can be wrong or at least confusing for the outer function.

This can lead to horribly bad error message, like `Unable to end. Error : Success` which makes no sense, and which lead me to finding this issue.

errno should only be checked after functions that explicitly declare that errno will be set after they return.

Some set of functions that I found have this problem:

glusterd_create_status_file()

Errno checked incorrectly in: https://github.com/gluster/glusterfs/blob/dc4aa17e617b21d9faa00dc5048e3396bde63d95/xlators/mgmt/glusterd/src/glusterd-geo-rep.c#L3841 (multiple other occurrences in this file)

It seems that the errno here can virtually come from any function, such as gf_msg() at the end of glusterd_create_status_file() printing a debug message, so this seems very wrong.

glusterd_is_fuse_available()

Errno checked incorrectly in: https://github.com/gluster/glusterfs/blob/dc4aa17e617b21d9faa00dc5048e3396bde63d95/xlators/mgmt/glusterd/src/glusterd-geo-rep.c#L3616 (multiple other occurrences in this file, and one more occurrence in a different file)

The errno here is used to generate the error message ("Unable to open /dev/fuse %s", strerror(errno)), which is wrong because the errno doesn't come from the open() call in glusterd_is_fuse_available() in all cases, it can also come from the close() call.

runner_start()

Errno checked incorrectly in: https://github.com/gluster/glusterfs/blob/dc4aa17e617b21d9faa00dc5048e3396bde63d95/xlators/mgmt/glusterd/src/glusterd-geo-rep.c#L5181 (multiple other occurrences in this file)

The errno here can easily come from sys_close() at the end of runner_start(), thus having nothing to do with the error message that it is used in.

This function has a return code that should probably be used instead.

runner_end()

Errno checked incorrectly in: https://github.com/gluster/glusterfs/blob/dc4aa17e617b21d9faa00dc5048e3396bde63d95/xlators/mgmt/glusterd/src/glusterd-geo-rep.c#L5227

The errno here can easily come from sys_close() at the end of runner_end(), thus having nothing to do with the error message that it is used in.

This function has a return code that should probably be used instead.

gf_strdup()

Errno checked without documentation asserting that it is OK: https://github.com/gluster/glusterfs/blob/dc4aa17e617b21d9faa00dc5048e3396bde63d95/xlators/mgmt/glusterd/src/glusterd-geo-rep.c#L5756 (multiple other occurrences in this file)

This use is probably not incorrect, but it doesn't gf_strdup() document that it guarantees to set a sensible errno, so that should not be relied upon, or such documentation should be added to the header file declaring gf_strdup().

Summary:

* 4 of the above functions seem to use errno in a way that can yield wrong or misleading error messages, thus producing error messages like `Unable to end. Error : Success`.

Comment 1 Shyamsundar 2018-06-20 18:26:43 UTC
This bug reported is against a version of Gluster that is no longer maintained (or has been EOL'd). See https://www.gluster.org/release-schedule/ for the versions currently maintained.

As a result this bug is being closed.

If the bug persists on a maintained version of gluster or against the mainline gluster repository, request that it be reopened and the Version field be marked appropriately.


Note You need to log in before you can comment on or make changes to this bug.