[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 21 Jun 2014 13:41:02 +0000
From: Isaac Dunham <ibid.ag@...il.com>
To: musl@...ts.openwall.com
Subject: Re: [PATCH v3] Implement fmtmsg
On Sat, Jun 21, 2014 at 12:46:26AM -0400, Rich Felker wrote:
> On Fri, Jun 20, 2014 at 07:32:01AM -0700, Isaac Dunham wrote:
> > Here's a third revision of fmtmsg.
>
> Thanks, it looks good. I'm doing a bit of cleanup before I commit --
> mostly things like mismatches spaces/tabs in indention/alignment,
> trailing spaces, etc. Also taking out the useless if around the while
> loop that contains the if condition as part of the while condition,
> debloating the header a bit (all the hex constants), adding protection
> against pthread cancellation, and fixing one error in the header:
> 0xffffffff is not a valid value for MM_NOTOK since it doesn't fit in
> int.
>
> I'm a bit tired now so rather than just commit and possibly have
> stupid mistakes, I'm sending my draft revisions here (attached). If
> you get a chance please take a look and see if they look ok.
All your changes look good, but I wrote one line that's unneeded.
(see below).
Also, I thought there was an issue with my code in _strcolcmp() for a moment,
since
_strcolcmp("label:sevarity","label:severity") == 1;
But that's not an issue, since
one side is always null-terminated at the end of the current component
(we compare to msgs[i]).
> Thanks!
>
> Rich
Thanks,
Isaac Dunham
> /* Public domain fmtmsg()
> * Written by Isaac Dunham, 2014
> */
> #include <fmtmsg.h>
> #include <fcntl.h>
> #include <unistd.h>
> #include <stdio.h>
> #include <string.h>
> #include <stdlib.h>
> #include <pthread.h>
>
> /*
> * If lstr is the first part of bstr, check that the next char in bstr
> * is either \0 or :
> */
> static int _strcolcmp(const char *lstr, const char *bstr)
> {
> size_t i = 0;
> while (lstr[i] && bstr[i] && (bstr[i] == lstr[i])) i++;
> if ( lstr[i] || (bstr[i] && bstr[i] != ':')) return 1;
> return 0;
> }
>
> int fmtmsg(long classification, const char *label, int severity,
> const char *text, const char *action, const char *tag)
> {
> int ret = 0, i, consolefd = 0, verb = 0;
> char *errstring = MM_NULLSEV, *cmsg = getenv("MSGVERB");
> char *const msgs[] = {
> "label", "severity", "text", "action", "tag", NULL
> };
> int cs;
>
> pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs);
>
> if (classification & MM_CONSOLE)
> consolefd = open("/dev/console", O_WRONLY);
> if (consolefd < 0) {
> classification &= ~MM_CONSOLE;
I don't think this is needed.
After all, MM_CONSOLE is not checked after this.
> ret = MM_NOCON;
> consolefd = 0;
> }
> if (severity == MM_HALT) errstring = "HALT: ";
> else if (severity == MM_ERROR) errstring = "ERROR: ";
> else if (severity == MM_WARNING) errstring = "WARNING: ";
> else if (severity == MM_INFO) errstring = "INFO: ";
> if (consolefd) {
> if (dprintf(consolefd, "%s%s%s%s%s%s%s%s\n",
> label?label:"", label?": ":"",
> severity?errstring:"", text?text:"",
> action?"\nTO FIX: ":"",
> action?action:"", action?" ":"", tag?tag:"" )<1)
> ret = MM_NOCON;
> }
>
> while (cmsg && cmsg[0]) {
> for(i=0; msgs[i]; i++) {
> if (!_strcolcmp(msgs[i], cmsg)) break;
> }
> if (msgs[i] == NULL) {
> //ignore MSGVERB-unrecognized component
> verb =0xFF;
> break;
> } else {
> verb |= (1 << i);
> cmsg = strchr(cmsg, ':');
> if (cmsg) cmsg++;
> }
> }
> if (!verb) verb = 0xFF;
> if (classification & MM_PRINT) {
> if (dprintf(2, "%s%s%s%s%s%s%s%s\n",
> (verb&1 && label) ? label : "",
> (verb&1 && label) ? ": " : "",
> (verb&2 && severity) ? errstring : "",
> (verb&4 && text) ? text : "",
> (verb&8 && action) ? "\nTO FIX: " : "",
> (verb&8 && action) ? action : "",
> (verb&8 && action) ? " " : "",
> (verb&16 && tag) ? tag : "" ) < 1)
> ret |= MM_NOMSG;
> }
> if ((ret & (MM_NOCON|MM_NOMSG)) == (MM_NOCON|MM_NOMSG))
> ret = MM_NOTOK;
>
> pthread_setcancelstate(cs, 0);
>
> return ret;
> }
> #ifndef _FMTMSG_H
> #define _FMTMSG_H
>
> #ifdef __cplusplus
> extern "C" {
> #endif
>
> #define MM_HARD 1
> #define MM_SOFT 2
> #define MM_FIRM 4
>
> #define MM_APPL 8
> #define MM_UTIL 16
> #define MM_OPSYS 32
>
> #define MM_RECOVER 64
> #define MM_NRECOV 128
>
> #define MM_PRINT 256
> #define MM_CONSOLE 512
>
> #define MM_NULLMC 0L
>
> #define MM_HALT 1
> #define MM_ERROR 2
> #define MM_WARNING 3
> #define MM_INFO 4
> #define MM_NOSEV 0
>
> #define MM_OK 0
> #define MM_NOTOK (-1)
> #define MM_NOMSG 1
> #define MM_NOCON 4
>
> #define MM_NULLLBL ((char*)0)
> #define MM_NULLTXT ((char*)0)
> #define MM_NULLACT ((char*)0)
> #define MM_NULLTAG ((char*)0)
> #define MM_NULLSEV 0
>
> int fmtmsg(long, const char *, int, const char *, const char *, const char *);
>
> #ifdef __cplusplus
> }
> #endif
>
> #endif
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
