Re: [PATCH] compaudit: avoid defining a global getent fallback

[Xichen Zeng <xichenzr@xxxxxxxxx>]

Hi Daniel,

Thanks for your review and suggestions.

> Have you considered renaming the local «getent» function to, e.g.,
> _compaudit_internal__getent, and then update the callsites to use that?

Both callsites only query groups, so I renamed the helper to 
_compaudit_get_group and narrowed its interface to a group name or GID.

> This introduces an assumption that $LOGNAME is a valid group name (or
> number), which is not correct on all systems. E.g., it's not correct on
> www.zsh.org.

The original code already calls `getent group $LOGNAME`. My 
understanding is that this checks for the user-private group special 
case: if no same-named group exists, the lookup fails and the exemption 
is not applied. I preserved that behavior and the original callsite 
branches, removing the intermediate _i_group variable. Please let me 
know if I have missed another issue.

> Shouldn't this codepath also be taken when a «getent» function is 
defined?

Agreed. The helper now uses an existing getent command or function when 
available, falling back to /etc/group otherwise.

I also added braces around the parameter expansions and changed the GID 
match to count fields from the start of the line, as suggested.

The revised patch is attached.

Thank you,

Xichen Zeng

diff --git a/Completion/compaudit b/Completion/compaudit
index 93a043124..8630972d3 100644
--- a/Completion/compaudit
+++ b/Completion/compaudit
@@ -15,13 +15,13 @@ compaudit() {                           # Define and then call
 emulate -L zsh
 setopt extendedglob
 
-[[ -n $commands[getent] ]] || getent() {
-  if [[ $1 = hosts ]]; then
-    sed 's/#.*//' /etc/$1 | grep -w $2
-  elif [[ $2 = <-> ]]; then
-    grep ":$2:[^:]*$" /etc/$1
+_compaudit_get_group() {
+  if (( $+commands[getent] || $+functions[getent] )); then
+    getent group "$1"
+  elif [[ $1 = <-> ]]; then
+    grep "^[^:]*:[^:]*:${1}:" /etc/group
   else
-    grep "^$2:" /etc/$1
+    grep "^${1}:" /etc/group
   fi
 }
 
@@ -134,9 +134,9 @@ _i_wdirs=( ${^fpath}(N-f:g+w:,-f:o+w:,-^${_i_owners})
 if (( $#_i_wdirs )); then
   local GROUP GROUPMEM _i_pw _i_gid
   if ((UID == EUID )); then
-    getent group $LOGNAME | IFS=: read GROUP _i_pw _i_gid GROUPMEM
+    _compaudit_get_group "$LOGNAME" | IFS=: read GROUP _i_pw _i_gid GROUPMEM
   else
-    getent group $EGID | IFS=: read GROUP _i_pw _i_gid GROUPMEM
+    _compaudit_get_group "$EGID" | IFS=: read GROUP _i_pw _i_gid GROUPMEM
   fi
 
   if [[ $GROUP == $LOGNAME && ( -z $GROUPMEM || $GROUPMEM == $LOGNAME ) ]]