This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]