This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] login: Replace macro-based control flow with function calls in utmp
* Adhemerval Zanella:
>> +/* file_locking_failed (LOCKING, FD, TYPE) returns true if the locking
>> + operation failed and recovery needs to be performed.
>> + (file_locking_restore (LOCKING) still needs to be called.)
>
> Punctuation seems strange here.
Sorry, I don't see it. How so?
>> +static bool
>> +file_locking_failed (struct file_locking *locking, int fd, int type)
>> +{
>> + /* Cancel any existing alarm. */
>> + locking->old_timeout = alarm (0);
>> +
>> + /* Establish signal handler. */
>> + struct sigaction action;
>> + action.sa_handler = timeout_handler;
>> + __sigemptyset (&action.sa_mask);
>> + action.sa_flags = 0;
>> + __sigaction (SIGALRM, &action, &locking->old_action);
>> +
>> + alarm (TIMEOUT);
>> +
>> + /* Try to get the lock. */
>> + struct flock fl =
>> + {
>> + .l_type = type,
>> + fl.l_whence = SEEK_SET,
>> + };
>> + return __fcntl64_nocancel (fd, F_SETLKW, &fl) < 0;
>> +}
>
> The name with ending '_failed' sound confusing, why not just
> 'file_locking_bool' ?
The name reflects the use, like this:
>> + struct file_locking fl;
>> + if (file_locking_failed (&fl, file_fd, F_RDLCK))
>> + nbytes = 0;
>> + else
>> {
>> + /* Read the next entry. */
>> + nbytes = __read_nocancel (file_fd, &last_entry, sizeof (struct utmp));
>> + file_locking_unlock (file_fd);
>> }
Should I call it try_file_lock, still with true for the failure case?
Thanks,
Florian