]> git.ipfire.org Git - thirdparty/shadow.git/commitdiff
lib/: Remove incorrect /*@out@*/ comment from functions that read the pointee
authorAlejandro Colomar <alx@kernel.org>
Mon, 11 Dec 2023 13:01:38 +0000 (14:01 +0100)
committerSerge Hallyn <serge@hallyn.com>
Mon, 15 Jan 2024 19:14:28 +0000 (13:14 -0600)
These functions (e.g., gr_free()), explicitly dereference the pointer
and read the pointee.

The /@out@/ comment, which is (almost) analogous to the
[[gnu::access(write_only, ...)]] attribute, means that the pointee can
be uninitialized, since it won't read it.  There's a difference between
/@out@/ and the GCC attribute: the attribute doesn't require that the
call writes to the pointee, while /@out@/ requires that the pointee be
fully initialized after the call, so it _must_ write to it.

A guess of why it was used is that these functions are similar to
free(3), which does not read the memory it frees, and so one would
assume that if it doesn't read, write_only (or equivalents) are good.
That's wrong in several ways:

-  free(3) does not read _nor_ write to the memory, so it would
   be slightly inappropriate to use write_only with it.  It wouldn't be
   "wrong", but [[gnu::access(none, ...)]] would be more appropriate.

-  Because /@out@/ requires that the call writes to the pointee, it
   would be wrong to use it in free(3), which doesn't write to the
   pointee.

-  Our functions are similar to free(3) conceptually, but they don't
   behave like free(3), since they do read the memory (pointee) (and
   also write to it), and thus they're actually read_write.

Link: <https://splint.org/manual/manual.html#undefined>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
lib/commonio.h
lib/groupio.c
lib/groupmem.c
lib/prototypes.h
lib/pwio.c
lib/pwmem.c
lib/sgroupio.c
lib/shadowio.c
lib/shadowmem.c
lib/subordinateio.c

index d5354bfbec41b3b78e380d760c42c7bca906cdd8..7ea82d45e954630a1b44e012f581d29c1e02e140 100644 (file)
@@ -37,7 +37,7 @@ struct commonio_ops {
        /*
         * free() the object including any strings pointed by it.
         */
-       void (*free) (/*@out@*/ /*@only@*/void *);
+       void (*free)(/*@only@*/void *);
 
        /*
         * Return the name of the object (for example, pw_name
index d9d9823bb67fe77dfc4f30bbdee6f2c308eafb85..7b9d45f2ca69d84f0688431ef28ce10af928d3ff 100644 (file)
@@ -36,7 +36,8 @@ static /*@null@*/ /*@only@*/void *group_dup (const void *ent)
        return __gr_dup (gr);
 }
 
-static void group_free (/*@out@*/ /*@only@*/void *ent)
+static void
+group_free(/*@only@*/void *ent)
 {
        struct group *gr = ent;
 
index eb3348aa53465765b22b77be401009b23a58d5dd..69d4435be5807f4b1067565c0a18685d97e1b1f7 100644 (file)
@@ -77,7 +77,8 @@ void gr_free_members (struct group *grent)
        }
 }
 
-void gr_free (/*@out@*/ /*@only@*/struct group *grent)
+void
+gr_free(/*@only@*/struct group *grent)
 {
        free (grent->gr_name);
        if (NULL != grent->gr_passwd) {
index 756bcdd6656a4746e48688c67a670658d77b485f..804ad96fe97024a1cae99afaa1e0a4e174f1676c 100644 (file)
@@ -188,7 +188,7 @@ extern void __gr_set_changed (void);
 /* groupmem.c */
 extern /*@null@*/ /*@only@*/struct group *__gr_dup (const struct group *grent);
 extern void gr_free_members (struct group *grent);
-extern void gr_free (/*@out@*/ /*@only@*/struct group *grent);
+extern void gr_free(/*@only@*/struct group *grent);
 
 /* hushed.c */
 extern bool hushed (const char *username);
@@ -355,7 +355,7 @@ extern /*@dependent@*/ /*@null@*/struct commonio_entry *__pw_get_head (void);
 
 /* pwmem.c */
 extern /*@null@*/ /*@only@*/struct passwd *__pw_dup (const struct passwd *pwent);
-extern void pw_free (/*@out@*/ /*@only@*/struct passwd *pwent);
+extern void pw_free(/*@only@*/struct passwd *pwent);
 
 /* csrand.c */
 unsigned long csrand (void);
@@ -418,7 +418,7 @@ extern struct spwd *sgetspent (const char *string);
 /* sgroupio.c */
 extern void __sgr_del_entry (const struct commonio_entry *ent);
 extern /*@null@*/ /*@only@*/struct sgrp *__sgr_dup (const struct sgrp *sgent);
-extern void sgr_free (/*@out@*/ /*@only@*/struct sgrp *sgent);
+extern void sgr_free(/*@only@*/struct sgrp *sgent);
 extern /*@dependent@*/ /*@null@*/struct commonio_entry *__sgr_get_head (void);
 extern void __sgr_set_changed (void);
 
@@ -428,7 +428,7 @@ extern void __spw_del_entry (const struct commonio_entry *ent);
 
 /* shadowmem.c */
 extern /*@null@*/ /*@only@*/struct spwd *__spw_dup (const struct spwd *spent);
-extern void spw_free (/*@out@*/ /*@only@*/struct spwd *spent);
+extern void spw_free(/*@only@*/struct spwd *spent);
 
 /* shell.c */
 extern int shell (const char *file, /*@null@*/const char *arg, char *const envp[]);
index 95ed0abcc1dd242f465319faf95d46edafec0562..3497c7545e9f7805660d2b1da70145ed53ace6e1 100644 (file)
@@ -26,7 +26,8 @@ static /*@null@*/ /*@only@*/void *passwd_dup (const void *ent)
        return __pw_dup (pw);
 }
 
-static void passwd_free (/*@out@*/ /*@only@*/void *ent)
+static void
+passwd_free(/*@only@*/void *ent)
 {
        struct passwd *pw = ent;
 
index 3d868a850b7c7c6e3afe68f9bca807f3f4c2d694..9c6e58d7e80ae3999b3be92a675c9f357b392e02 100644 (file)
@@ -70,7 +70,8 @@
        return pw;
 }
 
-void pw_free (/*@out@*/ /*@only@*/struct passwd *pwent)
+void
+pw_free(/*@only@*/struct passwd *pwent)
 {
        if (pwent != NULL) {
                free (pwent->pw_name);
index cbbdff7889d58d5e7a11b6b4157761b22bc732b0..0297df4ae4b93eeeb0686bd039e7514d5580f122 100644 (file)
@@ -117,14 +117,16 @@ static /*@null@*/ /*@only@*/void *gshadow_dup (const void *ent)
        return __sgr_dup (sg);
 }
 
-static void gshadow_free (/*@out@*/ /*@only@*/void *ent)
+static void
+gshadow_free(/*@only@*/void *ent)
 {
        struct sgrp *sg = ent;
 
        sgr_free (sg);
 }
 
-void sgr_free (/*@out@*/ /*@only@*/struct sgrp *sgent)
+void
+sgr_free(/*@only@*/struct sgrp *sgent)
 {
        size_t i;
        free (sgent->sg_name);
index 494ef7c8a7c232e95a0cce48393ba1e35305c63e..d2c3b4730b76e11b42027214e46e2d19799c0bb8 100644 (file)
@@ -31,7 +31,8 @@ static /*@null@*/ /*@only@*/void *shadow_dup (const void *ent)
        return __spw_dup (sp);
 }
 
-static void shadow_free (/*@out@*//*@only@*/void *ent)
+static void
+shadow_free(/*@only@*/void *ent)
 {
        struct spwd *sp = ent;
 
index c3c7c4cc34d50a6ba66be6ac8031600ac410bd87..9d8f193b251f01e430e1f3adfacb1592a660477b 100644 (file)
@@ -56,7 +56,8 @@
        return sp;
 }
 
-void spw_free (/*@out@*/ /*@only@*/struct spwd *spent)
+void
+spw_free(/*@only@*/struct spwd *spent)
 {
        if (spent != NULL) {
                free (spent->sp_namp);
index d67d56c91767e9688bff9515877e0f67135bb5a5..844de5f063519598b81908f7fbf2286c30afda9f 100644 (file)
@@ -56,7 +56,8 @@ static /*@null@*/ /*@only@*/void *subordinate_dup (const void *ent)
  *
  * @ent: pointer to a subordinate_range struct to free.
  */
-static void subordinate_free (/*@out@*/ /*@only@*/void *ent)
+static void
+subordinate_free(/*@only@*/void *ent)
 {
        struct subordinate_range *rangeent = ent;