Hi Frank, On Wed, 2021-12-01 at 10:23 -0500, Frank Ch. Eigler wrote: > > Although unlikely the MHD_add_response_header can fail for > > various reasons. If it fails something odd is going on. > > So check we can actually add a response header and log an > > error if we cannot. > > TBH I wouldn't bother even this much checking. It just uglifies the > code. If it would make covscan happier, I'd stick a (void) in front > of the add-header calls.
That is really just like ignoring the issue imho. But you are right that it uglifies the code, I'll wrap the calls in an helper function. > > - MHD_add_response_header (r, "Content-Type", "text/plain"); > > - MHD_RESULT rc = MHD_queue_response (c, code, r); > > + MHD_RESULT rc1, rc2; > > + rc1 = MHD_add_response_header (r, "Content-Type", > > "text/plain"); > > + rc2 = MHD_queue_response (c, code, r); > > MHD_destroy_response (r); > > - return rc; > > + return (rc1 == MHD_NO || rc2 == MHD_NO) ? MHD_NO : MHD_YES; > > e.g. this part won't work: returning MHD_NO causes libmicrohttpd to > send a 503 error back to the caller, regardless of our intended one. OK, so we only want to report MHD_NO here when MHD_queue_response fails. > > + if (MHD_add_response_header (resp, "Last-Modified", > > datebuf) == MHD_NO) > > + if (verbose) > > + obatched(clog) << "Error: couldn't add Last-Modified > > header" > > + << endl; > > } > > e.g., we normally report errors to the logs, regardless of verbosity > settings. OK, I'll remove the if (verbose). > > + if (MHD_add_response_header (r, "Content-Type", > > + "application/octet-stream") == > > MHD_NO > > + || MHD_add_response_header (r, "X-DEBUGINFOD-SIZE", > > + to_string(s.st_size).c_str() > > ) == MHD_NO > > + || MHD_add_response_header (r, "X-DEBUGINFOD-FILE", > > + file.c_str()) == MHD_NO) > > e.g., this formulation makes it impossible to add some headers if a > previous one failed. It is likely that if one fails, then all others fail similarly, but I see your point. Any header is better than no headers at all. Thanks, Mark
