URL:
https://github.com/SSSD/sssd/pull/844
Title: #844: sssd-ad and gpo_child: GPO apply fixes during reading fails
pbrezina commented:
"""
```diff
+static errno_t
+gpo_cache_remove_file(const char *smb_path,
+ const char *smb_cse_suffix)
...
+ ret = unlink(filename);
+ if (ret == -1) {
+ if (ENOENT == errno) {
+ ret = EOK;
+ } else {
+ ret = errno;
+ DEBUG(SSSDBG_CRIT_FAILURE, "failed to unlink %s\n", filename);
+ goto done;
+ }
+ }
```
I'd rather see something like:
```c
if (ret != 0) {
if (errno != ENOENT) {
ret = errno;
DEBUG(SSSDBG_CRIT_FAILURE, "failed to unlink %s [%s]: %s\n", filename,
ret, sss_strerror(ret);
goto done;
}
}
ret = EOK;
done:
...
```
Please, keep `if` on one line or include parentheses:
```diff
@@ -702,7 +702,7 @@ perform_smb_operations(int cached_gpt_version,
{
...
- done:
+ if (_sysvol_gpt_version >= 0)
+ ret = EOK;
+
smbc_free_context(smbc_ctx, 0);
return ret;
}
```
Otherwise it looks good code-wise. @mzidek-rh can you please review it function-wise?
Thank you.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/844#issuecomment-532597034