Do some more cleanup based on Coverity scan results.

michaelrsweet · Sep 11, 2024 · 13e4133 · 13e4133
1 parent bd48f58
commit 13e4133
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 14 deletions.
diff --git a/pappl/client-auth.c b/pappl/client-auth.c
@@ -60,12 +60,27 @@ http_status_t				// O - HTTP status
 papplClientIsAuthorized(
     pappl_client_t *client)		// I - Client
 {
+  char	admin_group[256];		// Admin group name
+  gid_t	admin_gid;			// Admin group ID
+
+
   // Range check input...
   if (!client)
     return (HTTP_STATUS_BAD_REQUEST);
 
   // Authorize for admin access...
-  return (_papplClientIsAuthorizedForGroup(client, false, client->system->admin_group, client->system->admin_gid));
+  _papplRWLockRead(client->system);
+
+  if (client->system->admin_group)
+    papplCopyString(admin_group, client->system->admin_group, sizeof(admin_group));
+  else
+    admin_group[0] = '\0';
+
+  admin_gid = client->system->admin_gid;
+
+  _papplRWUnlock(client->system);
+
+  return (_papplClientIsAuthorizedForGroup(client, false, admin_group, admin_gid));
 }
 
 

diff --git a/pappl/job-ipp.c b/pappl/job-ipp.c
@@ -238,9 +238,6 @@ _papplJobCopyDocumentData(
     client->system->clean_time = time(NULL) + 60;
   _papplRWUnlock(client->system);
 
-  _papplRWUnlock(job);
-  _papplRWUnlock(client->printer);
-
   ra = cupsArrayNew((cups_array_cb_t)strcmp, NULL, NULL, 0, NULL, NULL);
   cupsArrayAdd(ra, "job-id");
   cupsArrayAdd(ra, "job-state");
@@ -249,6 +246,9 @@ _papplJobCopyDocumentData(
 
   _papplJobCopyAttributesNoLock(job, client, ra);
   cupsArrayDelete(ra);
+
+  _papplRWUnlock(job);
+  _papplRWUnlock(client->printer);
 }
 
 
@@ -674,9 +674,13 @@ ipp_get_job_attributes(
 
   papplClientRespondIPP(client, IPP_STATUS_OK, NULL);
 
+  _papplRWLockRead(job);
+
   ra = ippCreateRequestedArray(client->request);
   _papplJobCopyAttributesNoLock(job, client, ra);
   cupsArrayDelete(ra);
+
+  _papplRWUnlock(job);
 }
 
 

diff --git a/pappl/job.c b/pappl/job.c
@@ -793,6 +793,8 @@ _papplJobSubmitFile(
     pappl_job_t *job,			// I - Job
     const char  *filename)		// I - Filename
 {
+  _papplRWLockWrite(job);
+
   if (!job->format)
   {
     // Open the file
@@ -862,10 +864,16 @@ _papplJobSubmitFile(
       // Process the job...
       job->state = IPP_JSTATE_PENDING;
 
+      _papplRWUnlock(job);
       _papplRWLockWrite(job->printer);
       _papplPrinterCheckJobsNoLock(job->printer);
       _papplRWUnlock(job->printer);
     }
+    else
+    {
+      // Queue the job...
+      _papplRWUnlock(job);
+    }
   }
   else
   {
@@ -876,6 +884,8 @@ _papplJobSubmitFile(
     job->state     = IPP_JSTATE_ABORTED;
     job->completed = time(NULL);
 
+    _papplRWUnlock(job);
+
     papplLogJob(job, PAPPL_LOGLEVEL_ERROR, "Unable to allocate filename.");
 
     if (!strncmp(filename, job->system->directory, dirlen) && filename[dirlen] == '/')

diff --git a/pappl/printer-webif.c b/pappl/printer-webif.c
@@ -946,7 +946,7 @@ _papplPrinterWebIteratorCallback(
 
   papplClientHTMLPrintf(client,
 			"          <p><img class=\"%s\" src=\"%s/icon-md.png\">%s, %s", ippEnumString("printer-state", (int)printer_state), printer->uriname, localize_keyword(client, "printer-state", state_str, text, sizeof(text)), jobs_str);
-  if ((printer->system->options & PAPPL_SOPTIONS_MULTI_QUEUE) && printer->printer_id == printer->system->default_printer_id)
+  if ((printer->system->options & PAPPL_SOPTIONS_MULTI_QUEUE) && printer->printer_id == papplSystemGetDefaultPrinterID(printer->system))
     papplClientHTMLPrintf(client, ", %s", papplClientGetLocString(client, _PAPPL_LOC("default printer")));
   if (printer->hold_new_jobs)
     papplClientHTMLPrintf(client, ", %s", papplClientGetLocString(client, _PAPPL_LOC("holding new jobs")));

diff --git a/pappl/system-accessors.c b/pappl/system-accessors.c
@@ -1570,6 +1570,9 @@ papplSystemSetAdminGroup(
 // - `HTTP_STATUS_FORBIDDEN` if the authentication succeeded but the user is
 //   not part of the specified group.
 //
+// > Note: The authentication callback can only be set prior to calling
+// > @link papplSystemRun@.
+//
 
 void
 papplSystemSetAuthCallback(
@@ -1578,16 +1581,12 @@ papplSystemSetAuthCallback(
     pappl_auth_cb_t auth_cb,		// I - Callback function
     void            *auth_cbdata)	// I - Callback data
 {
-  if (system)
+  if (system && !system->is_running)
   {
-    _papplRWLockWrite(system);
-
     free(system->auth_scheme);
     system->auth_scheme = auth_scheme ? strdup(auth_scheme) : NULL;
     system->auth_cb     = auth_cb;
     system->auth_cbdata = auth_cbdata;
-
-    _papplRWUnlock(system);
   }
 }
 
@@ -1742,12 +1741,8 @@ papplSystemSetFooterHTML(
 {
   if (system && html && !system->is_running)
   {
-    _papplRWLockWrite(system);
-
     free(system->footer_html);
     system->footer_html = strdup(html);
-
-    _papplRWUnlock(system);
   }
 }