Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Posix return_code variables could be initialized to ERROR to simplify code #1386

Open
2 tasks done
thnkslprpt opened this issue May 4, 2023 · 0 comments · May be fixed by #1387
Open
2 tasks done

Posix return_code variables could be initialized to ERROR to simplify code #1386

thnkslprpt opened this issue May 4, 2023 · 0 comments · May be fixed by #1387

Comments

@thnkslprpt
Copy link
Contributor

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
A few return_code variables in the POSIX implementation units could be initialized to ERROR/FAILURE which would negate the need to assign at each failure point.

Code snips

int32 OS_BinSemCreate_Impl(const OS_object_token_t *token, uint32 initial_value, uint32 options)
{
int ret;
int attr_created;
int mutex_created;
int cond_created;
int32 return_code;
pthread_mutexattr_t mutex_attr;
OS_impl_binsem_internal_record_t *sem;
/*
* This preserves a bit of pre-existing functionality that was particular to binary sems:
* if the initial value is greater than 1 it just silently used 1 without error.
* (by contrast the counting semaphore will return an error)
*/
if (initial_value > 1)
{
initial_value = 1;
}
attr_created = 0;
mutex_created = 0;
cond_created = 0;
sem = OS_OBJECT_TABLE_GET(OS_impl_bin_sem_table, *token);
memset(sem, 0, sizeof(*sem));
do
{
/*
** Initialize the pthread mutex attribute structure with default values
*/
ret = pthread_mutexattr_init(&mutex_attr);
if (ret != 0)
{
OS_DEBUG("Error: pthread_mutexattr_init failed: %s\n", strerror(ret));
return_code = OS_SEM_FAILURE;
break;
}
/* After this point, the attr object should be destroyed before return */
attr_created = 1;
/*
** Use priority inheritance
*/
ret = pthread_mutexattr_setprotocol(&mutex_attr, PTHREAD_PRIO_INHERIT);
if (ret != 0)
{
OS_DEBUG("Error: pthread_mutexattr_setprotocol failed: %s\n", strerror(ret));
return_code = OS_SEM_FAILURE;
break;
}
/*
** Initialize the mutex that is used with the condition variable
*/
ret = pthread_mutex_init(&(sem->id), &mutex_attr);
if (ret != 0)
{
OS_DEBUG("Error: pthread_mutex_init failed: %s\n", strerror(ret));
return_code = OS_SEM_FAILURE;
break;
}
mutex_created = 1;
/*
** Initialize the condition variable
*/
ret = pthread_cond_init(&(sem->cv), NULL);
if (ret != 0)
{
OS_DEBUG("Error: pthread_cond_init failed: %s\n", strerror(ret));
return_code = OS_SEM_FAILURE;
break;
}
cond_created = 1;
/*
* Check sem call, avoids unreachable destroy logic
*/
ret = pthread_cond_signal(&(sem->cv));
if (ret != 0)
{
OS_DEBUG("Error: initial pthread_cond_signal failed: %s\n", strerror(ret));
return_code = OS_SEM_FAILURE;
break;
}
/*
** fill out the proper OSAL table fields
*/
sem->current_value = initial_value;
return_code = OS_SUCCESS;
} while (0);
/* Clean up resources if the operation failed */
if (return_code != OS_SUCCESS)
{
if (mutex_created)
{
pthread_mutex_destroy(&(sem->id));
}
if (cond_created)
{
pthread_cond_destroy(&(sem->cv));
}
}
if (attr_created)
{
/* Done with the attribute object -
* this call is a no-op in linux - but for other implementations if
* the create call allocated something this should free it
*/
pthread_mutexattr_destroy(&mutex_attr);
}
return return_code;
}

Same for OS_Posix_TableMutex_Init() and OS_Posix_TimeBaseAPI_Impl_Init().

Expected behavior
Simplify code by setting default to ERROR/FAILURE and assigning SUCCESS when necessary.

Reporter Info
Avi Weiss @thnkslprpt

thnkslprpt added a commit to thnkslprpt/osal that referenced this issue May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants