[hackers] [st] [PATCH] Makefile: always output the build options, even when building with install
--- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 470ac86407bb..6eb2d9d04852 100644 --- a/Makefile +++ b/Makefile @@ -40,7 +40,7 @@ dist: clean tar -cf - st-$(VERSION) | gzip > st-$(VERSION).tar.gz rm -rf st-$(VERSION) -install: st +install: all mkdir -p $(DESTDIR)$(PREFIX)/bin cp -f st $(DESTDIR)$(PREFIX)/bin chmod 755 $(DESTDIR)$(PREFIX)/bin/st -- 2.40.0
Re: [hackers] [st][PATCH] Fix xresources bgcolour fgcolour and cscolour definitions
Hi Hari, I *highly* doubt that there is any interest in getting this into mainline. There is already a patch which adds this behaviour. -- Best Regards, Tom Schwindl
Re: [hackers] [dwm] [PATCH] add a comment to clarify a potential overflow of ltsymbol
Hi Hiltjo, > Hi, > > I think it is too verbose and don't think the comment is neccesary. > I can understand the verbosity part, but that's something I can change. My point here was more that bug reports get posted to the ML and I even get them in private or from people I know in person. So I think a comment, as you, yourself suggested, might be helpful. A simple "Hey, if you're exceeding 16 bytes we cannot guarantee for anything" would be enough (not in that tone, of course) for me. -- Best Regards, Tom Schwindl > -- > Kind regards, > Hiltjo
[hackers] [dwm] [PATCH] add a comment to clarify a potential overflow of ltsymbol
In case the strncpy() call is advised to copy >=16 characters, ltsymbol
overflows.
As dwm does not expect to have a ltsymbol bigger than 15 characters, there will
be
no length check[0]. Our target audience are programmers, they should be able
to figure out how to extend the length by themselves. The reason to add this
comment
is that some inexperienced users are easily confused by this and the topic
comes up
from time to time[1].
[0] https://lists.suckless.org/hackers/2208/18484.html
[1] https://lists.suckless.org/dev/2210/35000.html
---
I know that this patch looks like bikeshedding, but the comment is convenient.
I also got messaged in private a few(!) times about that exact issue, but
I'm not sure if I'm allowed to quote these mails here. Anyways, the point is
that this issue arises from time to time and a comment would make our/their
lifes easier.
---
dwm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/dwm.c b/dwm.c
index c2bd8710544e..e400aa028587 100644
--- a/dwm.c
+++ b/dwm.c
@@ -396,6 +396,9 @@ arrange(Monitor *m)
void
arrangemon(Monitor *m)
{
+ /* dwm supports a ltsymbol size up to 15 chars (plus terminating
NUL-byte).
+* Anything greater than that will cause issues. If the user wants a
name
+* containing more chars, they need to modify the code to fit their
needs */
strncpy(m->ltsymbol, m->lt[m->sellt]->symbol, sizeof m->ltsymbol);
if (m->lt[m->sellt]->arrange)
m->lt[m->sellt]->arrange(m);
--
2.39.1
Re: [hackers] [dwm] [PATCH] use correct conversion specifier for an unsigned integer
> > I think %d is fine and correct here too. > > -- > Kind regards, > Hiltjo Indeed. I was wondering about this since it looks inconsistent to me. But to be fair, in reality this will probably never be problem. -- Best Regards, Tom Schwindl
[hackers] [dwm] [PATCH] use correct conversion specifier for an unsigned integer
--- dwm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dwm.c b/dwm.c index 03baf42b3502..d832678ab5aa 100644 --- a/dwm.c +++ b/dwm.c @@ -1121,7 +1121,7 @@ monocle(Monitor *m) if (ISVISIBLE(c)) n++; if (n > 0) /* override layout symbol */ - snprintf(m->ltsymbol, sizeof m->ltsymbol, "[%d]", n); + snprintf(m->ltsymbol, sizeof m->ltsymbol, "[%u]", n); for (c = nexttiled(m->clients); c; c = nexttiled(c->next)) resize(c, m->wx, m->wy, m->ww - 2 * c->bw, m->wh - 2 * c->bw, 0); } -- 2.39.0
[hackers] [sent] [PATCH] remove unnecessary NULL checks and add `void` for an empty parameter list
---
I sent (hehe) this a long time ago, it might be of interest now that some
work is done.
---
sent.c | 30 ++
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/sent.c b/sent.c
index 9534fcaf97b9..49853ee6c273 100644
--- a/sent.c
+++ b/sent.c
@@ -99,12 +99,12 @@ static void load(FILE *fp);
static void advance(const Arg *arg);
static void quit(const Arg *arg);
static void resize(int width, int height);
-static void run();
-static void usage();
-static void xdraw();
-static void xhints();
-static void xinit();
-static void xloadfonts();
+static void run(void);
+static void usage(void);
+static void xdraw(void);
+static void xhints(void);
+static void xinit(void);
+static void xloadfonts(void);
static void bpress(XEvent *);
static void cmessage(XEvent *);
@@ -216,8 +216,7 @@ ffload(Slide *s)
s->img->bufwidth = ntohl(*(uint32_t *)[8]);
s->img->bufheight = ntohl(*(uint32_t *)[12]);
- if (s->img->buf)
- free(s->img->buf);
+ free(s->img->buf);
/* internally the image is stored in 888 format */
s->img->buf = ecalloc(s->img->bufwidth * s->img->bufheight,
strlen("888"));
@@ -496,7 +495,7 @@ resize(int width, int height)
}
void
-run()
+run(void)
{
XEvent ev;
@@ -518,7 +517,7 @@ run()
}
void
-xdraw()
+xdraw(void)
{
unsigned int height, width, i;
Image *im = slides[idx].img;
@@ -546,7 +545,7 @@ xdraw()
}
void
-xhints()
+xhints(void)
{
XClassHint class = {.res_name = "sent", .res_class = "presenter"};
XWMHints wm = {.flags = InputHint, .input = True};
@@ -564,7 +563,7 @@ xhints()
}
void
-xinit()
+xinit(void)
{
XTextProperty prop;
unsigned int i;
@@ -608,7 +607,7 @@ xinit()
}
void
-xloadfonts()
+xloadfonts(void)
{
int i, j;
char *fstrs[LEN(fontfallbacks)];
@@ -627,8 +626,7 @@ xloadfonts()
}
for (j = 0; j < LEN(fontfallbacks); j++)
- if (fstrs[j])
- free(fstrs[j]);
+ free(fstrs[j]);
}
void
@@ -677,7 +675,7 @@ configure(XEvent *e)
}
void
-usage()
+usage(void)
{
die("usage: %s [file]", argv0);
}
--
2.39.0
Re: [hackers] Re: [sbase][PATCH] dd: fix for ibs * count < obs
Hi,
On Thu Dec 22, 2022 at 2:00 AM CET, phoebos wrote:
> I thought this message had gone through but it doesn't appear in the
> mailing list archive. I added some context to my patch below, and would
> appreciate any feedback or confirmation that the email got through?
>
> Thanks
>
> On Tue, Nov 29, 2022 at 22:27:59 +, phoebos wrote:
> > Perhaps I should add some context.
> >
> > Running:
> >
> > yes | dd ibs=1 count=1
> >
> > reports that 512 bytes are read:
> >
> > 512+0 records in
> > 1+0 records out
> >
> > Only one byte should be read, as occurs with GNU and busybox dd:
> >
> > 1+0 records in
> > 0+1 records out
> >
> > I believe the reason for this bug in sbase is because of the comparison
> > with obs in the below loop, which overlooks cases such as these.
> >
> > All feedback is welcome. Is this an appropriate fix to the bug?
> >
> > phoebos
> >
> > On Tue, Nov 22, 2022 at 16:28:35 +, phoebos wrote:
> > > Previously, running `dd ibs=1 count=1` read 512 bytes rather than 1.
> > > ---
> > > dd.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/dd.c b/dd.c
> > > index 6061048..4081eca 100644
> > > --- a/dd.c
> > > +++ b/dd.c
> > > @@ -174,7 +174,7 @@ main(int argc, char *argv[])
> > > /* XXX: handle non-seekable files */
> > > }
> > > while (!eof && (count == -1 || ifull + ipart < count)) {
> > > - while (ipos - opos < obs) {
> > > + while (ipos - opos < obs && ifull + ipart < count) {
> > > ret = read(ifd, buf + ipos, ibs);
> > > if (ret == 0) {
> > > eof = 1;
> > > --
> > > 2.38.1
> > >
Your previous message went through, at least for me.
The archive is a bit buggy and sometimes misses some mails.
As for your diff, sbase is pretty inactive and I don't know how often a
maintainer checks the mailing list for patches.
--
Best Regards,
Tom Schwindl
Re: [hackers] [sites][PATCH] Adding the "movekeyboard" dwm(6.4) patch, which adds the ability to move floating windows on the x and y axis through a keybinding/keyboard instead of only the mouse.
On Sat Dec 17, 2022 at 10:45 PM CET, mrmine1 wrote: > Hi, > > sorry for the inconvenience, this is my first commit. Thanks for the > corrections. > > Best Regards > I just saw that you already sent it to the wiki. Apparantly, it was already pushed to the website[1]. [1] https://dwm.suckless.org/patches/movekeyboard/ -- Best Regards, Tom Schwindl
Re: [hackers] [sites][PATCH] Adding the "movekeyboard" dwm(6.4) patch, which adds the ability to move floating windows on the x and y axis through a keybinding/keyboard instead of only the mouse.
Hi, this should be commited to the wiki. See https://suckless.org/wiki/ Additionally, you shouldn't put the whole commit message into the subject of your patch. -- Best Regards, Tom Schwindl
Re: [hackers] [dwm][patch] xkb indicator patch for 6.4
Hi, this should be commited to the wiki. See https://suckless.org/wiki/ -- Best Regards, Tom Schwindl
Re: [hackers] [libgrapheme] Add a check make-target as an alias for test || Laslo Hunhold
Hi Laslo, On Sun Nov 20, 2022 at 11:42 PM CET, wrote: > commit a796095218b0524f957f76d6f3b501ebda700d44 > Author: Laslo Hunhold > AuthorDate: Tue Nov 15 21:08:50 2022 +0100 > Commit: Laslo Hunhold > CommitDate: Tue Nov 15 21:08:50 2022 +0100 > > Add a check make-target as an alias for test > > It's one extra line but helps a bit as the "community" seems to be a bit > split on how to call it (test or check). > > Signed-off-by: Laslo Hunhold > > diff --git a/Makefile b/Makefile > index e49b50c..93e0ce3 100644 > --- a/Makefile > +++ b/Makefile > @@ -308,6 +308,8 @@ $(MAN7:=.7): > benchmark: $(BENCHMARK) > for m in $(BENCHMARK); do ./$$m; done > > +check: test > + > test: $(TEST) > for m in $(TEST); do ./$$m; done > This should probably be added to the PHONY target as a prerequisite. -- Best Regards, Tom Schwindl
Re: [hackers] [sbase][PATCH 1/2] libutil: Implement a simple yes/no prompt
Hi Alan,
On Sun Nov 20, 2022 at 1:17 PM CET, Alan Potteiger wrote:
> On Sun, Nov 20, 2022 at 2:17 AM Sebastian LaVine wrote:
> >
> > This only reads a single character, not a line as the standard states.
> > If the user responds with 'y\n', then that '\n' is going to be read if
> > the program takes any more input. I suggest using getline(3) instead.
> >
> > [0]: https://www.man7.org/linux/man-pages/man1/cp.1p.html
> >
>
> My mistake. I misunderstood how the buffering works in stdio, I played
> with it and
> saw exactly what you mean. Switched to getline(3).
>
> ---
> Makefile | 1 +
> libutil/promptyn.c | 26 ++
> util.h | 1 +
> 3 files changed, 28 insertions(+)
> create mode 100644 libutil/promptyn.c
>
> diff --git a/Makefile b/Makefile
> index 3243b1c..0a5c930 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -63,6 +63,7 @@ LIBUTILSRC =\
> libutil/mkdirp.c\
> libutil/mode.c\
> libutil/parseoffset.c\
> + libutil/promptyn.c\
> libutil/putword.c\
> libutil/reallocarray.c\
> libutil/recurse.c\
> diff --git a/libutil/promptyn.c b/libutil/promptyn.c
> new file mode 100644
> index 000..b63bfaa
> --- /dev/null
> +++ b/libutil/promptyn.c
> @@ -0,0 +1,26 @@
> +/* See LICENSE file for copyright and license details. */
> +#include
> +#include
> +
> +int
> +promptyn(const char *nomsg, const char *promptfmt, ...)
> +{
> + va_list ap;
> + char *linep = NULL;
> + size_t linecapp = 1;
> +
> + va_start(ap, promptfmt);
> + vfprintf(stderr, promptfmt, ap);
> + va_end(ap);
> +
> + fprintf(stderr, " (y/n [n]): ");
> + getline(, , stdin);
> + switch (*linep) {
> + case 'y': case 'Y':
> + return 0;
> + default:
> + fprintf(stderr, "%s\n", nomsg);
> + return 1;
> + }
> +}
> +
You should check if getline(3) failed before dereferencing the pointer.
linecapp should be zero (or not initialized) since its a throw away variable.
linep should be freed.
I don't quite get why there is a variable argument list instead of just using
a single argument which contains the prompt. I can't see the need for formatting
in a y/n prompt. I also wouldn't use `nomsg' since it just polutes stderr.
Additionally, you should fix your style in this diff.
I'm not sure if we even _want_ to implement the `-i' option.
Someone with more knowledge about the sbase design history could elaborate a bit
further on this.
--
Best Regards,
Tom Schwindl
Re: [hackers] [lchat] Makefile: add dist target to create release tarballs || Jan Klemkow
Hi Laslo, On Fri Oct 21, 2022 at 9:35 AM CEST, Laslo Hunhold wrote: > On Thu, 20 Oct 2022 19:18:42 -0400 > Steve Ward wrote: > > Dear Steve, > > > If you want to stick with git, the mkdir, cp, tar, and rm commands > > could be replaced with: > > git archive --prefix lchat-$(VERSION)/ HEAD | gzip > > > lchat-$(VERSION).tar.gz > > this would add an implicit dependency on git, though. > git already is a dependency, given the following line. > + cp -r $$(git ls-tree --name-only HEAD) lchat-$(VERSION) -- Best Regards, Tom Schwindl
[hackers] [dmenu] [PATCH] dmenu: use die() to print the usage message
---
dmenu.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/dmenu.c b/dmenu.c
index 571bc3558014..999f71fa91a1 100644
--- a/dmenu.c
+++ b/dmenu.c
@@ -710,9 +710,8 @@ setup(void)
static void
usage(void)
{
- fputs("usage: dmenu [-bfiv] [-l lines] [-p prompt] [-fn font] [-m
monitor]\n"
- " [-nb color] [-nf color] [-sb color] [-sf color] [-w
windowid]\n", stderr);
- exit(1);
+ die("usage: dmenu [-bfiv] [-l lines] [-p prompt] [-fn font] [-m
monitor]\n"
+ " [-nb color] [-nf color] [-sb color] [-sf color] [-w
windowid]");
}
int
--
2.37.3
Re: [hackers] [libgrapheme] Update to Unicode 15.0.0 || Laslo Hunhold
Hi Laslo, I think the README has to be updated, too. It still states that libgrapheme conforms to Unicode 14.0.0. -- Best Regards, Tom Schwindl
Re: [hackers] [PATCH] [st] FAQ: libXft now supports BGRA glyphs
Hi, On Thu Sep 15, 2022 at 3:45 PM CEST, NRK wrote: > On Thu, Sep 15, 2022 at 01:31:02PM +0000, Tom Schwindl wrote: > > -## BadLength X error in Xft when trying to render emoji > > - > > -Xft makes st crash when rendering color emojis with the following error: > > - > > -"X Error of failed request: BadLength (poly request too large or internal > > Xlib length error)" > > - Major opcode of failed request: 139 (RENDER) > > - Minor opcode of failed request: 20 (RenderAddGlyphs) > > - Serial number of failed request: 1595 > > - Current serial number in output stream: 1818" > > - > > -This is a known bug in Xft (not st) which happens on some platforms and > > -combination of particular fonts and fontconfig settings. > > - > > -See also: > > -https://gitlab.freedesktop.org/xorg/lib/libxft/issues/6 > > -https://bugs.freedesktop.org/show_bug.cgi?id=107534 > > -https://bugzilla.redhat.com/show_bug.cgi?id=1498269 > > I don't think removing the entire section is the right idea, since there > are distros which are still on older versions. > > > -The solution is to remove color emoji fonts or disable this in the > > fontconfig > > -XML configuration. As an ugly workaround (which may work only on newer > > -fontconfig versions (FC_COLOR)), the following code can be used to mask > > color > > -fonts: > > Simply changing this paragraph to say that the solution is to upgrade > libXft should be the proper way to go IMO. > > - NRK Thanks for the suggestion. I attached another patch which does this. I also removed the last sentence, given that it's fixed now. -- Best Regards, Tom Schwindl From 619604a617c8146044f68a2280f0792e2a78830f Mon Sep 17 00:00:00 2001 From: Tom Schwindl Date: Thu, 15 Sep 2022 16:08:43 +0200 Subject: [PATCH] FAQ: libXft now supports BGRA glyphs https://gitlab.freedesktop.org/xorg/lib/libxft/-/blob/libXft-2.3.5/NEWS --- FAQ | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/FAQ b/FAQ index 969b195ebcff..b84abfec3ee2 100644 --- a/FAQ +++ b/FAQ @@ -239,12 +239,8 @@ https://gitlab.freedesktop.org/xorg/lib/libxft/issues/6 https://bugs.freedesktop.org/show_bug.cgi?id=107534 https://bugzilla.redhat.com/show_bug.cgi?id=1498269 -The solution is to remove color emoji fonts or disable this in the fontconfig -XML configuration. As an ugly workaround (which may work only on newer -fontconfig versions (FC_COLOR)), the following code can be used to mask color -fonts: +This was fixed in libXft-2.3.5 [1]. To get the correct behavior, you need to +upgrade to this or a newer version. - FcPatternAddBool(fcpattern, FC_COLOR, FcFalse); +[1] https://gitlab.freedesktop.org/xorg/lib/libxft/-/blob/libXft-2.3.5/NEWS -Please don't bother reporting this bug to st, but notify the upstream Xft -developers about fixing this bug. -- 2.37.3
[hackers] [PATCH] [st] FAQ: libXft now supports BGRA glyphs
https://gitlab.freedesktop.org/xorg/lib/libxft/-/blob/libXft-2.3.5/NEWS --- FAQ | 28 1 file changed, 28 deletions(-) diff --git a/FAQ b/FAQ index 969b195ebcff..e36b173361d5 100644 --- a/FAQ +++ b/FAQ @@ -220,31 +220,3 @@ diff --git a/x.c b/x.c dc.col[IS_SET(MODE_REVERSE)? defaultfg : defaultbg].pixel); - -## BadLength X error in Xft when trying to render emoji - -Xft makes st crash when rendering color emojis with the following error: - -"X Error of failed request: BadLength (poly request too large or internal Xlib length error)" - Major opcode of failed request: 139 (RENDER) - Minor opcode of failed request: 20 (RenderAddGlyphs) - Serial number of failed request: 1595 - Current serial number in output stream: 1818" - -This is a known bug in Xft (not st) which happens on some platforms and -combination of particular fonts and fontconfig settings. - -See also: -https://gitlab.freedesktop.org/xorg/lib/libxft/issues/6 -https://bugs.freedesktop.org/show_bug.cgi?id=107534 -https://bugzilla.redhat.com/show_bug.cgi?id=1498269 - -The solution is to remove color emoji fonts or disable this in the fontconfig -XML configuration. As an ugly workaround (which may work only on newer -fontconfig versions (FC_COLOR)), the following code can be used to mask color -fonts: - - FcPatternAddBool(fcpattern, FC_COLOR, FcFalse); - -Please don't bother reporting this bug to st, but notify the upstream Xft -developers about fixing this bug. -- 2.37.3
[hackers] [ii] [PATCH] ii: Remove unnecessary explicit zeroing of variables
---
ii.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/ii.c b/ii.c
index 0856e8f010d3..520f75d1618a 100644
--- a/ii.c
+++ b/ii.c
@@ -32,7 +32,7 @@ size_t strlcpy(char *, const char *, size_t);
#define IRC_MSG_MAX 512 /* guaranteed to be <= than PIPE_BUF */
#define PING_TIMEOUT 600
-enum { TOK_NICKSRV = 0, TOK_USER, TOK_CMD, TOK_CHAN, TOK_ARG, TOK_TEXT,
TOK_LAST };
+enum { TOK_NICKSRV, TOK_USER, TOK_CMD, TOK_CHAN, TOK_ARG, TOK_TEXT, TOK_LAST };
typedef struct Channel Channel;
struct Channel {
@@ -76,9 +76,9 @@ static int udsopen(const char *);
static void usage(void);
static int isrunning = 1;
-static time_t last_response = 0;
-static Channel *channels = NULL;
-static Channel *channelmaster = NULL;
+static time_t last_response;
+static Channel *channels;
+static Channel *channelmaster;
static char nick[32]; /* active nickname at runtime */
static char _nick[32]; /* nickname at startup */
static char ircpath[PATH_MAX]; /* irc dir (-i) */
@@ -106,7 +106,7 @@ usage(void)
static void
ewritestr(int fd, const char *s)
{
- size_t len, off = 0;
+ size_t len, off;
int w = -1;
len = strlen(s);
@@ -405,7 +405,7 @@ isnumeric(const char *s)
static size_t
tokenize(char **result, size_t reslen, char *str, int delim)
{
- char *p = NULL, *n = NULL;
+ char *p, *n;
size_t i = 0;
for (n = str; *n == ' '; n++)
@@ -433,7 +433,7 @@ tokenize(char **result, size_t reslen, char *str, int delim)
static void
channel_print(Channel *c, const char *buf)
{
- FILE *fp = NULL;
+ FILE *fp;
time_t t = time(NULL);
if (!(fp = fopen(c->outpath, "a")))
@@ -454,7 +454,7 @@ proc_channels_privmsg(int ircfd, Channel *c, char *buf)
static void
proc_channels_input(int ircfd, Channel *c, char *buf)
{
- char *p = NULL;
+ char *p;
size_t buflen;
if (buf[0] == '\0')
@@ -548,7 +548,7 @@ proc_server_cmd(int fd, char *buf)
{
Channel *c;
const char *channel;
- char *argv[TOK_LAST], *cmd = NULL, *p = NULL;
+ char *argv[TOK_LAST], *cmd, *p;
unsigned int i;
if (!buf || buf[0] == '\0')
@@ -667,7 +667,7 @@ static int
read_line(int fd, char *buf, size_t bufsiz)
{
size_t i = 0;
- char c = '\0';
+ char c;
do {
if (read(fd, , sizeof(char)) != sizeof(char))
--
2.37.2
Re: [hackers] [ii][PATCH] use square brackets for optional -u parameter in man page
Hi, On Tue Aug 30, 2022 at 10:43 AM CEST, Petr Vaněk wrote: > --- > ii.1 | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/ii.1 b/ii.1 > index 8e06af7..db05ab6 100644 > --- a/ii.1 > +++ b/ii.1 > @@ -29,8 +29,8 @@ and ii creates a new channel directory with in and out file. > .IR nickname ] > .RB [ \-f > .IR realname ] > -.RB < \-u > -.IR sockname > > +.RB [ \-u > +.IR sockname ] > .SH OPTIONS > .TP > .BI \-s " servername" > -- > 2.35.1 While we're at it, I think we should also remove the angle brackets around the `-s' option and sync the names of the arguments with the ones used in the usage() function. Additionally, the `-u' option is missing in the usage message and a lot of unnecessary angle brackets are used, too. -- Best Regards, Tom Schwindl
Re: [hackers] [sbase] [PATCH] printf: Do not read past the end of the format string
> Here, we enter the loop with `i = formatlen'. After the dot (`.') is matched, > `i' is increased by one and effectively "overflows". This should actually be: Here, after the dot (`.') is matched, the same thing as in the previous case happens. `i' is matched three times instead of the expected two and thus effectively "overflows". -- Best Regards, Tom Schwindl
[hackers] [sbase] [PATCH] printf: Do not read past the end of the format string
If a trailing `%' character occurs, we read past the end of the format
string and thus introduce UB. Reproducible by executing the following:
./printf %
This happens because the format string here actually consists of two
characters, `%' _and_ the trailing nul-byte. The flag parsing loop
matches the nul-byte with `0' and thus increases the counter, `i', to
be `formatlen + 1'. Furthermore, `i' is used as an index to access the
format string, which will eventually lead to an out-of-bounds access.
This can be fixed by simply checking the value of `i' like this:
if (i > formatlen) eprintf(...);
However, there are two more ways in which `i' can "overflow".
The second "overflow" could happen after parsing the "field width".
Given the following call,
./printf %42
we enter the loop with `i = 1'. The condition is matched _3_ times, again,
because of the trailing nul-byte. `i' now has a value of `formatlen + 1'.
This can be fixed by using the same check as above.
The last way in which `i' might "overflow" is after parsing the "field
precision".
Take this call as an example:
./printf %.42
Here, we enter the loop with `i = formatlen'. After the dot (`.') is matched,
`i' is increased by one and effectively "overflows".
The fix for this is the same as above. I agree that it's a bit ugly to
repeat this check three times, but the alternatives seem to add more cruft.
Note that the last two cases only appear if numbers are used after the
percent sign. Other characters don't match the conditions and just
fall through to the `switch' statements default case, which emits an error.
---
Although I've tested this, it's not unlikely that I've overseen something.
As stated above, this solution isn't beautiful, but after playing around
a bit with different approaches, it turned out to be the most convenient
(I'm happy to be corrected on this). Anyways, it's definitely better than
having UB in a tool which is marked as "finished".
---
printf.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/printf.c b/printf.c
index 039dac717105..75667dad8d36 100644
--- a/printf.c
+++ b/printf.c
@@ -54,6 +54,9 @@ main(int argc, char *argv[])
flag = format[i];
}
+ if (i > formatlen)
+ eprintf("Missing conversion specifier.\n");
+
/* field width */
width = -1;
if (format[i] == '*') {
@@ -74,6 +77,9 @@ main(int argc, char *argv[])
}
}
+ if (i > formatlen)
+ eprintf("Missing conversion specifier.\n");
+
/* field precision */
precision = -1;
if (format[i] == '.') {
@@ -96,6 +102,9 @@ main(int argc, char *argv[])
}
}
+ if (i > formatlen)
+ eprintf("Missing conversion specifier.\n");
+
if (format[i] != '%') {
if (argi < argc)
arg = argv[argi++];
--
2.37.2
[hackers] [st] [PATCH] st: use `void' to indicate an empty parameter list
---
st.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/st.c b/st.c
index 6ba467d7cc90..62def59f17cb 100644
--- a/st.c
+++ b/st.c
@@ -939,7 +939,7 @@ ttyresize(int tw, int th)
}
void
-ttyhangup()
+ttyhangup(void)
{
/* Send SIGHUP to shell */
kill(pid, SIGHUP);
--
2.37.1
Re: [hackers] [sbase] [PATCH] Use ar(1)'s s-flag instead of invoking ranlib(1)
Hi, On Tue Aug 16, 2022 at 5:47 PM CEST, Laslo Hunhold wrote: > On Mon, 1 Aug 2022 11:09:16 +0200 > "Roberto E. Vargas Caballero" wrote: > > Dear Roberto, > > > Because then you will support only the last systems. If you keep > > the ranlib you will support systems that support all versions of > > the standard. Again, if you find a system without ranlib then > > we can talk and consider what to do, but removing only for the sake > > of "the standard does not include anymore ranlib" is a horrible idea. > > For example, scc requires the use of ranlib, if you remove it then > > I will not be able to continue testing scc with suckless software. > > What happens if I want to compile sbase in an old SunOs workstation? > > I thought about it a bit more in the last few weeks and added ranlib > again. > > The main reason is that I find it convincing that POSIX would not try > to define varying binary formats, which is why the toolchain-tool > ranlib(1) was probably never included. > > Adding the s-flag to ar is simply an unexpected and ill-fitting > feature-creep that bloats up an otherwise simple archive-tool. > > Thanks for this very interesting discussion and sharing your > experience! > > With best regards > > Laslo It was indeed interesting. We should also undo the libzahl patch then. -- Best Regards, Tom Schwindl
[hackers] [libgrapheme] [PATCH] Remove dead file `src/util.c'
Since commit 072bb271868a3583da1f ("Introduce mostly branchless character
break detection") removed the code from the file, it no longer serves
a purpose.
---
Makefile | 2 --
src/util.c | 8
2 files changed, 10 deletions(-)
delete mode 100644 src/util.c
diff --git a/Makefile b/Makefile
index 1eb9507bb752..ac73ab9b1d0e 100644
--- a/Makefile
+++ b/Makefile
@@ -45,7 +45,6 @@ SRC =\
src/line\
src/sentence\
src/utf8\
- src/util\
src/word\
TEST =\
@@ -87,7 +86,6 @@ src/character.o: src/character.c config.mk gen/character.h
grapheme.h src/util.h
src/line.o: src/line.c config.mk gen/line.h grapheme.h src/util.h
src/sentence.o: src/sentence.c config.mk gen/sentence.h grapheme.h src/util.h
src/utf8.o: src/utf8.c config.mk grapheme.h
-src/util.o: src/util.c config.mk gen/types.h grapheme.h src/util.h
src/word.o: src/word.c config.mk gen/word.h grapheme.h src/util.h
test/character.o: test/character.c config.mk gen/character-test.h grapheme.h
test/util.h
test/line.o: test/line.c config.mk gen/line-test.h grapheme.h test/util.h
diff --git a/src/util.c b/src/util.c
deleted file mode 100644
index 7b8e176e535d..
--- a/src/util.c
+++ /dev/null
@@ -1,8 +0,0 @@
-/* See LICENSE file for copyright and license details. */
-#include
-#include
-#include
-
-#include "../gen/types.h"
-#include "../grapheme.h"
-#include "util.h"
--
2.37.1
[hackers] [ii] [PATCH] ii: Add a die() function to replace fprintf(3) + exit(3) calls
---
ii.c | 92 +++-
1 file changed, 41 insertions(+), 51 deletions(-)
diff --git a/ii.c b/ii.c
index 2d485aa3e453..0c9ae74cb174 100644
--- a/ii.c
+++ b/ii.c
@@ -56,6 +56,7 @@ static int channel_reopen(Channel *);
static void channel_rm(Channel *);
static void create_dirtree(const char *);
static void create_filepath(char *, size_t, const char *, const char *,
const char *);
+static void die(const char *, ...);
static void ewritestr(int, const char *);
static void handle_channels_input(int, Channel *);
static void handle_server_output(int);
@@ -83,13 +84,23 @@ static char _nick[32]; /* nickname at startup */
static char ircpath[PATH_MAX]; /* irc dir (-i) */
static char msg[IRC_MSG_MAX]; /* message buf used for communication */
+static void
+die(const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ vfprintf(stderr, fmt, ap);
+ va_end(ap);
+ exit(1);
+}
+
static void
usage(void)
{
- fprintf(stderr, "usage: %s <-s host> [-i ] [-p ] "
+ die("usage: %s <-s host> [-i ] [-p ] "
"[-u ] [-n ] [-k ] "
"[-f ]\n", argv0);
- exit(1);
}
static void
@@ -103,10 +114,8 @@ ewritestr(int fd, const char *s)
if ((w = write(fd, s + off, len - off)) == -1)
break;
}
- if (w == -1) {
- fprintf(stderr, "%s: write: %s\n", argv0, strerror(errno));
- exit(1);
- }
+ if (w == -1)
+ die("%s: write: %s\n", argv0, strerror(errno));
}
/* creates directories bottom-up, if necessary */
@@ -184,8 +193,7 @@ create_filepath(char *filepath, size_t len, const char
*path,
return;
error:
- fprintf(stderr, "%s: path to irc directory too long\n", argv0);
- exit(1);
+ die("%s: path to irc directory too long\n", argv0);
}
static int
@@ -229,10 +237,8 @@ channel_new(const char *name)
strlcpy(channelpath, name, sizeof(channelpath));
channel_normalize_path(channelpath);
- if (!(c = calloc(1, sizeof(Channel {
- fprintf(stderr, "%s: calloc: %s\n", argv0, strerror(errno));
- exit(1);
- }
+ if (!(c = calloc(1, sizeof(Channel
+ die("%s: calloc: %s\n", argv0, strerror(errno));
strlcpy(c->name, name, sizeof(c->name));
channel_normalize_name(c->name);
@@ -341,21 +347,17 @@ udsopen(const char *uds)
size_t len;
int fd;
- if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
- fprintf(stderr, "%s: socket: %s\n", argv0, strerror(errno));
- exit(1);
- }
+ if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1)
+ die("%s: socket: %s\n", argv0, strerror(errno));
sun.sun_family = AF_UNIX;
- if (strlcpy(sun.sun_path, uds, sizeof(sun.sun_path)) >=
sizeof(sun.sun_path)) {
- fprintf(stderr, "%s: UNIX domain socket path truncation\n",
argv0);
- exit(1);
- }
+ if (strlcpy(sun.sun_path, uds, sizeof(sun.sun_path)) >=
sizeof(sun.sun_path))
+ die("%s: UNIX domain socket path truncation\n", argv0);
+
len = strlen(sun.sun_path) + 1 + sizeof(sun.sun_family);
- if (connect(fd, (struct sockaddr *), len) == -1) {
- fprintf(stderr, "%s: connect: %s\n", argv0, strerror(errno));
- exit(1);
- }
+ if (connect(fd, (struct sockaddr *), len) == -1)
+ die("%s: connect: %s\n", argv0, strerror(errno));
+
return fd;
}
@@ -370,10 +372,8 @@ tcpopen(const char *host, const char *service)
hints.ai_flags = AI_NUMERICSERV; /* avoid name lookup for port */
hints.ai_socktype = SOCK_STREAM;
- if ((e = getaddrinfo(host, service, , ))) {
- fprintf(stderr, "%s: getaddrinfo: %s\n", argv0,
gai_strerror(e));
- exit(1);
- }
+ if ((e = getaddrinfo(host, service, , )))
+ die("%s: getaddrinfo: %s\n", argv0, gai_strerror(e));
for (rp = res; rp; rp = rp->ai_next) {
fd = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol);
@@ -386,11 +386,9 @@ tcpopen(const char *host, const char *service)
}
break; /* success */
}
- if (fd == -1) {
- fprintf(stderr, "%s: could not connect to %s:%s: %s\n",
+ if (fd == -1)
+ die("%s: could not connect to %s:%s: %s\n",
argv0, host, service, strerror(errno));
- exit(1);
- }
freeaddrinfo(res);
return fd;
@@ -705,11 +703,9 @@ handle_server_output(int ircfd)
{
char buf[IRC_MSG_MAX];
- if (read_line(ircfd, buf, sizeof(buf)) == -1) {
- fprintf(stderr, "%s: remote host closed connection: %s\n",
- argv0,
[hackers] [libzahl] [PATCH] Use ar(1)'s s-flag instead of invoking ranlib(1)
ranlib(1) is legacy and not even part of POSIX anymore, ar(1) can do the same job with the s-flag (which is an XSI-extension, but whatever). --- Makefile | 3 +-- config.mk | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/Makefile b/Makefile index bd6e47237c5f..b8852583fb93 100644 --- a/Makefile +++ b/Makefile @@ -140,8 +140,7 @@ all: libzahl.a $(DOC) $(CC) $(CFLAGS) $(CPPFLAGS) -c -o $@ $< libzahl.a: $(OBJ) - $(AR) rc $@ $? - $(RANLIB) $@ + $(AR) -rcs $@ $? test-random.c: test-generate.py ./test-generate.py > test-random.c diff --git a/config.mk b/config.mk index ad9c5c26da94..2a42d3cbba27 100644 --- a/config.mk +++ b/config.mk @@ -9,7 +9,6 @@ DOCPREFIX = $(PREFIX)/share/doc CC = cc AR = ar -RANLIB = ranlib CPPFLAGS = -D_DEFAULT_SOURCE -D_BSD_SOURCE -D_XOPEN_SOURCE=700 -DGOOD_RAND CFLAGS = -std=c99 -O3 -flto -Wall -pedantic -- 2.37.1
[hackers] [sbase] [PATCH] Use ar(1)'s s-flag instead of invoking ranlib(1)
ranlib(1) is legacy and not even part of POSIX anymore, ar(1) can do the same job with the s-flag (which is an XSI-extension, but whatever). --- shamelessly stole Laslos commit message --- Makefile | 6 ++ config.mk | 1 - 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 3243b1cfacef..ee2e7fea1cba 100644 --- a/Makefile +++ b/Makefile @@ -202,12 +202,10 @@ $(OBJ): $(HDR) config.mk $(CC) $(CFLAGS) $(CPPFLAGS) -o $@ -c $< $(LIBUTF): $(LIBUTFOBJ) - $(AR) rc $@ $? - $(RANLIB) $@ + $(AR) -rcs $@ $? $(LIBUTIL): $(LIBUTILOBJ) - $(AR) rc $@ $? - $(RANLIB) $@ + $(AR) -rcs $@ $? getconf.o: getconf.h diff --git a/config.mk b/config.mk index 9fb18da1463c..a03fe29b9c8f 100644 --- a/config.mk +++ b/config.mk @@ -7,7 +7,6 @@ MANPREFIX = $(PREFIX)/share/man CC = cc AR = ar -RANLIB = ranlib # for NetBSD add -D_NETBSD_SOURCE # -lrt might be needed on some systems -- 2.37.1
[hackers] [lsw] [PATCH] Improve compliance with our coding style
---
lsw.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/lsw.c b/lsw.c
index c7d5171e7383..497d6d262c1f 100644
--- a/lsw.c
+++ b/lsw.c
@@ -8,7 +8,7 @@
static Atom netwmname;
static Display *dpy;
-char *
+static char *
getname(Window win)
{
static char buf[BUFSIZ];
@@ -16,22 +16,23 @@ getname(Window win)
int n;
XTextProperty prop;
- if (!XGetTextProperty(dpy, win, , netwmname) || prop.nitems == 0)
- if (!XGetWMName(dpy, win, ) || prop.nitems == 0)
+ if (!XGetTextProperty(dpy, win, , netwmname) || !prop.nitems) {
+ if (!XGetWMName(dpy, win, ) || !prop.nitems)
return "";
+ }
if (!XmbTextPropertyToTextList(dpy, , , ) && n > 0) {
- strncpy(buf, list[0], sizeof buf);
+ strncpy(buf, list[0], sizeof(buf));
XFreeStringList(list);
} else {
- strncpy(buf, (char *)prop.value, sizeof buf);
+ strncpy(buf, (char *)prop.value, sizeof(buf));
}
XFree(prop.value);
- buf[sizeof buf - 1] = '\0';
+ buf[sizeof(buf) - 1] = '\0';
return buf;
}
-void
+static void
lsw(Window win)
{
unsigned int n;
@@ -40,10 +41,11 @@ lsw(Window win)
if (!XQueryTree(dpy, win, , , , ) || !n)
return;
- for (w = [n-1]; w >= [0]; w--)
+ for (w = [n-1]; w >= [0]; w--) {
if (XGetWindowAttributes(dpy, *w, )
&& !wa.override_redirect && wa.map_state == IsViewable)
printf("0x%07lx %s\n", *w, getname(*w));
+ }
XFree(wins);
}
--
2.37.0
Re: [hackers] [sent] [PATCH 3/3] Makefile: config.mk: Improve Makefile and config.mk
> Hi Tom o/
Hi Quentin \o/
> > > > # includes and libs
> > > > -INCS = -I. -I/usr/include -I/usr/include/freetype2 -I${X11INC}
> > > > -LIBS = -L/usr/lib -lc -lm -L${X11LIB} -lXft -lfontconfig -lX11
> > > > +INCS = -I/usr/include/freetype2 -I${X11INC}
> > > > +LIBS = -lm -L${X11LIB} -lXft -lfontconfig -lX11
> > >
> > > The -L option should go into LDFLAGS
> >
> > Since $LIBS is only used to provide $LDFLAGS, we could copy it's contents
> > directly into $LDFLAGS and drop $LIBS. Good catch.
>
> No, those are two different things.
>
> LDFLAGS are linker configuration parameters,
> like indeed where to search libraries amongst other things.
>
> The LIBS macro is to indicate what libraries to link against;
> those are not configuration parameters,
> but actual ordered operands in the linking chain.
Now that I'm looking over this again, I think the `-L` option can simply be
dropped. It isn't necessary in this context.
We could do it like surf (at least iirc) and drop $LDFLAGS in favour of
$LIBS. Let $LDFLAGS be an optional configuration option for the user.
> > > > # flags
> > > > CPPFLAGS = -DVERSION=\"${VERSION}\" -D_XOPEN_SOURCE=600
> > > > -CFLAGS += -g -std=c99 -pedantic -Wall ${INCS} ${CPPFLAGS}
> > > > -LDFLAGS += -g ${LIBS}
> > > > -#CFLAGS += -std=c99 -pedantic -Wall -Os ${INCS} ${CPPFLAGS}
> > > > -#LDFLAGS += ${LIBS}
> > > > +CFLAGS = -std=c99 -pedantic -Wall -Wstrict-prototypes
> > > > -Wold-style-definition -Os ${INCS} ${CPPFLAGS}
> > >
> > > The -std option isn't necessary with the default CC.
> > > I'd remove any warning from production build,
> > > warnings are for development,
> > > then developpers can set whatever warnings they like.
> > >
> > > > # compiler and linker
> > > > -CC ?= cc
> > > > +CC = cc
> > >
> > > This should be removed altogether if you want a standard Makefile,
> > > then CC will just be the expected c99.
> >
> > I'd keep the `-std` option and the CC assignment.
> > We want to be C99 compliant regardless of which compiler the user decides
> > to use.
> > Additionally, CC is a configuration option in all suckless programs and
> > to be consistent here seems reasonable, in my opinion.
>
> That's the point. To be compliant with C99, use a c99 compiler,
> which is the default compiler called by standard Makefiles.
> By defining CC to something non-standard,
> we introduce an indirection change here,
> that then forces the (non-standard) -std option to be passed to random
> compiler
> in the hope that it'll compatible with it.
> If the user decides to replace the standard compiler
> with another one, then they'll know which option to pass to it to make it
> respecting
> the original contract.
>
> As for some of the other suckless programs that specify CC,
> that's just a legacy format that should be replaced anyway IMO.
>
On most systems `c99` is just a simple wrapper script with, roughly, the
following content:
exec /usr/bin/cc -std=c99 "$@"
`-std` is already used here and we don't gain much by using the systems `c99`.
But in the case in which `c99` isn't a script (which we probably should assume),
your way is definitely the better one.
Luckily, that's something the maintainer has to decide.
Btw. does anyone know if he is still active on this list (or in general)?
--
Best Regards,
Tom Schwindl
Re: [hackers] [sent] [PATCH 3/3] Makefile: config.mk: Improve Makefile and config.mk
Hi Quentin,
> I think that you can also drop the || altogether,
> make will just tell you whatever problem it encountered
> while trying to invoque cscope.
>
> > - @mkdir -p ${DESTDIR}${MANPREFIX}/man1
> > - @cp sent.1 ${DESTDIR}${MANPREFIX}/man1/sent.1
> > - @chmod 644 ${DESTDIR}${MANPREFIX}/man1/sent.1
> > + mkdir -p ${DESTDIR}${MANPREFIX}/man1
> > + cp sent.1 ${DESTDIR}${MANPREFIX}/man1/sent.1
>
> I'd cp -f here too.
I agree.
> > # includes and libs
> > -INCS = -I. -I/usr/include -I/usr/include/freetype2 -I${X11INC}
> > -LIBS = -L/usr/lib -lc -lm -L${X11LIB} -lXft -lfontconfig -lX11
> > +INCS = -I/usr/include/freetype2 -I${X11INC}
> > +LIBS = -lm -L${X11LIB} -lXft -lfontconfig -lX11
>
> The -L option should go into LDFLAGS
Since $LIBS is only used to provide $LDFLAGS, we could copy it's contents
directly into $LDFLAGS and drop $LIBS. Good catch.
> > # flags
> > CPPFLAGS = -DVERSION=\"${VERSION}\" -D_XOPEN_SOURCE=600
> > -CFLAGS += -g -std=c99 -pedantic -Wall ${INCS} ${CPPFLAGS}
> > -LDFLAGS += -g ${LIBS}
> > -#CFLAGS += -std=c99 -pedantic -Wall -Os ${INCS} ${CPPFLAGS}
> > -#LDFLAGS += ${LIBS}
> > +CFLAGS = -std=c99 -pedantic -Wall -Wstrict-prototypes
> > -Wold-style-definition -Os ${INCS} ${CPPFLAGS}
>
> The -std option isn't necessary with the default CC.
> I'd remove any warning from production build,
> warnings are for development,
> then developpers can set whatever warnings they like.
>
> > # compiler and linker
> > -CC ?= cc
> > +CC = cc
>
> This should be removed altogether if you want a standard Makefile,
> then CC will just be the expected c99.
I'd keep the `-std` option and the CC assignment.
We want to be C99 compliant regardless of which compiler the user decides to
use.
Additionally, CC is a configuration option in all suckless programs and
to be consistent here seems reasonable, in my opinion.
--
Best Regards,
Tom Schwindl
Re: [hackers] [sent] [PATCH 3/3] Makefile: config.mk: Improve Makefile and config.mk
Hi Hiltjo, > I would remove the echo also for the verbose changes. I get the point of consistency, but I think an output as the following is more confusing than helpful. echo sent build options: sent build options: This seems to just pollute stdout with unnecessary information. I've even played with the idea of removing all echos and just keep the ones under the `options` target, because I don't think the others add much value. But I'll wait for the maintainers feedback before rewriting the whole Makefile. > I see the CFLAGS is also changed. You should document it in the commit > message. I agree. > I'm not sure I agree with adding -Wall or the many -Woptions though. > > It is up to the current sent maintainer to decide. Since most Suckless tools include a few `-W` options, NRK's suggestion was sound to me, but that's indeed a point the maintainer has to decide. And just for the record: `-Wall` was already there, I just added the other `-W` options. -- Best Regards, Tom Schwindl
[hackers] [sent] [PATCH 2/3] treewide: Improve compliance with our coding style
- Add `void` as parameter for functions which do not take any Arguments
- Use a block if the body of a loop consists of an `if` statement
- A few minor changes such as moving a curley bracket
---
drw.c | 3 ++-
sent.c | 69 +-
util.c | 3 ++-
3 files changed, 39 insertions(+), 36 deletions(-)
diff --git a/drw.c b/drw.c
index c1582e746cc5..84f87b972384 100644
--- a/drw.c
+++ b/drw.c
@@ -19,9 +19,10 @@ static const long utfmax[UTF_SIZ + 1] = {0x10, 0x7F,
0x7FF, 0x, 0x10
static long
utf8decodebyte(const char c, size_t *i)
{
- for (*i = 0; *i < (UTF_SIZ + 1); ++(*i))
+ for (*i = 0; *i < (UTF_SIZ + 1); ++(*i)) {
if (((unsigned char)c & utfmask[*i]) == utfbyte[*i])
return (unsigned char)c & ~utfmask[*i];
+ }
return 0;
}
diff --git a/sent.c b/sent.c
index 9534fcaf97b9..a27b704cd5ce 100644
--- a/sent.c
+++ b/sent.c
@@ -99,12 +99,12 @@ static void load(FILE *fp);
static void advance(const Arg *arg);
static void quit(const Arg *arg);
static void resize(int width, int height);
-static void run();
-static void usage();
-static void xdraw();
-static void xhints();
-static void xinit();
-static void xloadfonts();
+static void run(void);
+static void usage(void);
+static void xdraw(void);
+static void xhints(void);
+static void xinit(void);
+static void xloadfonts(void);
static void bpress(XEvent *);
static void cmessage(XEvent *);
@@ -116,12 +116,12 @@ static void configure(XEvent *);
#include "config.h"
/* Globals */
-static const char *fname = NULL;
-static Slide *slides = NULL;
-static int idx = 0;
-static int slidecount = 0;
+static const char *fname;
+static Slide *slides;
+static int idx;
+static int slidecount;
static XWindow xw;
-static Drw *d = NULL;
+static Drw *d;
static Clr *sc;
static Fnt *fonts[NUMFONTSCALES];
static int running = 1;
@@ -326,9 +326,10 @@ getfontsize(Slide *s, unsigned int *width, unsigned int
*height)
float lfac = linespacing * (s->linecount - 1) + 1;
/* fit height */
- for (j = NUMFONTSCALES - 1; j >= 0; j--)
+ for (j = NUMFONTSCALES - 1; j >= 0; j--) {
if (fonts[j]->h * lfac <= xw.uh)
break;
+ }
LIMIT(j, 0, NUMFONTSCALES - 1);
drw_setfontset(d, fonts[j]);
@@ -381,7 +382,7 @@ cleanup(int slidesonly)
void
reload(const Arg *arg)
{
- FILE *fp = NULL;
+ FILE *fp;
unsigned int i;
if (!fname) {
@@ -414,15 +415,17 @@ load(FILE *fp)
/* read each line from fp and add it to the item list */
while (1) {
/* eat consecutive empty lines */
- while ((p = fgets(buf, sizeof(buf), fp)))
+ while ((p = fgets(buf, sizeof(buf), fp))) {
if (strcmp(buf, "\n") != 0 && buf[0] != '#')
break;
+ }
if (!p)
break;
- if ((slidecount+1) * sizeof(*slides) >= size)
+ if ((slidecount+1) * sizeof(*slides) >= size) {
if (!(slides = realloc(slides, (size += BUFSIZ
die("sent: Unable to reallocate %u bytes:",
size);
+ }
/* read one slide */
maxlines = 0;
@@ -496,7 +499,7 @@ resize(int width, int height)
}
void
-run()
+run(void)
{
XEvent ev;
@@ -518,7 +521,7 @@ run()
}
void
-xdraw()
+xdraw(void)
{
unsigned int height, width, i;
Image *im = slides[idx].img;
@@ -528,15 +531,11 @@ xdraw()
if (!im) {
drw_rect(d, 0, 0, xw.w, xw.h, 1, 1);
- for (i = 0; i < slides[idx].linecount; i++)
- drw_text(d,
-(xw.w - width) / 2,
+ for (i = 0; i < slides[idx].linecount; i++) {
+ drw_text(d, (xw.w - width) / 2,
(xw.h - height) / 2 + i * linespacing *
d->fonts->h,
-width,
-d->fonts->h,
-0,
-slides[idx].lines[i],
-0);
+width, d->fonts->h, 0, slides[idx].lines[i],
0);
+ }
drw_map(d, xw.win, 0, 0, xw.w, xw.h);
} else {
if (!(im->state & SCALED))
@@ -546,7 +545,7 @@ xdraw()
}
void
-xhints()
+xhints(void)
{
XClassHint class = {.res_name = "sent", .res_class = "presenter"};
XWMHints wm = {.flags = InputHint, .input = True};
@@ -564,7 +563,7 @@ xhints()
}
void
-xinit()
+xinit(void)
{
XTextProperty prop;
unsigned int i;
@@ -608,14 +607,13 @@ xinit()
}
void
-xloadfonts()
+xloadfonts(void)
{
int i, j;
char *fstrs[LEN(fontfallbacks)];
- for (j = 0; j <
[hackers] [sent] [PATCH 1/3] sent.c: Drop unnecessary NULL checks
Since no operation is performed when free(3) is used on NULL, remove
the checks.
---
sent.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/sent.c b/sent.c
index eb0ac970b1f5..3916cb0c4ad1 100644
--- a/sent.c
+++ b/sent.c
@@ -216,8 +216,7 @@ ffload(Slide *s)
s->img->bufwidth = ntohl(*(uint32_t *)[8]);
s->img->bufheight = ntohl(*(uint32_t *)[12]);
- if (s->img->buf)
- free(s->img->buf);
+ free(s->img->buf);
/* internally the image is stored in 888 format */
s->img->buf = ecalloc(s->img->bufwidth * s->img->bufheight,
strlen("888"));
@@ -625,8 +624,7 @@ xloadfonts(void)
}
for (j = 0; j < LEN(fontfallbacks); j++)
- if (fstrs[j])
- free(fstrs[j]);
+ free(fstrs[j]);
}
void
--
2.36.1
[hackers] [sent] [PATCH 3/3] Makefile: config.mk: Improve Makefile and config.mk
Let the Makefile be a bit more verbose and remove unnecessary extensions
and flags in config.mk.
---
Makefile | 37 +++--
config.mk | 18 --
2 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/Makefile b/Makefile
index 56e636734216..d73909283970 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,6 @@
# sent - plain text presentation tool
# See LICENSE file for copyright and license details.
+.POSIX:
include config.mk
@@ -18,43 +19,43 @@ config.h:
cp config.def.h config.h
.c.o:
- @echo CC $<
- @${CC} -c ${CFLAGS} $<
+ @echo ${CC} $<
+ ${CC} -c ${CFLAGS} $<
${OBJ}: config.h config.mk
sent: ${OBJ}
- @echo CC -o $@
- @${CC} -o $@ ${OBJ} ${LDFLAGS}
+ @echo ${CC} -o $@
+ ${CC} -o $@ ${OBJ} ${LDFLAGS}
cscope: ${SRC} config.h
@echo cScope
- @cscope -R -b || echo cScope not installed
+ cscope -R -b || echo cScope not installed
clean:
@echo cleaning
- @rm -f sent ${OBJ} sent-${VERSION}.tar.gz
+ rm -f sent ${OBJ} sent-${VERSION}.tar.gz
dist: clean
@echo creating dist tarball
- @mkdir -p sent-${VERSION}
- @cp -R LICENSE Makefile config.mk config.def.h ${SRC} sent-${VERSION}
- @tar -cf sent-${VERSION}.tar sent-${VERSION}
- @gzip sent-${VERSION}.tar
- @rm -rf sent-${VERSION}
+ mkdir -p sent-${VERSION}
+ cp -R LICENSE Makefile config.mk config.def.h ${SRC} sent-${VERSION}
+ tar -cf sent-${VERSION}.tar sent-${VERSION}
+ gzip sent-${VERSION}.tar
+ rm -rf sent-${VERSION}
install: all
@echo installing executable file to ${DESTDIR}${PREFIX}/bin
- @mkdir -p ${DESTDIR}${PREFIX}/bin
- @cp -f sent ${DESTDIR}${PREFIX}/bin
- @chmod 755 ${DESTDIR}${PREFIX}/bin/sent
+ mkdir -p ${DESTDIR}${PREFIX}/bin
+ cp -f sent ${DESTDIR}${PREFIX}/bin
+ chmod 755 ${DESTDIR}${PREFIX}/bin/sent
@echo installing manual page to ${DESTDIR}${MANPREFIX}/man1
- @mkdir -p ${DESTDIR}${MANPREFIX}/man1
- @cp sent.1 ${DESTDIR}${MANPREFIX}/man1/sent.1
- @chmod 644 ${DESTDIR}${MANPREFIX}/man1/sent.1
+ mkdir -p ${DESTDIR}${MANPREFIX}/man1
+ cp sent.1 ${DESTDIR}${MANPREFIX}/man1/sent.1
+ chmod 644 ${DESTDIR}${MANPREFIX}/man1/sent.1
uninstall:
@echo removing executable file from ${DESTDIR}${PREFIX}/bin
- @rm -f ${DESTDIR}${PREFIX}/bin/sent
+ rm -f ${DESTDIR}${PREFIX}/bin/sent
.PHONY: all options clean dist install uninstall cscope
diff --git a/config.mk b/config.mk
index d61c55437f46..15fdcfa10dc7 100644
--- a/config.mk
+++ b/config.mk
@@ -11,20 +11,18 @@ X11INC = /usr/X11R6/include
X11LIB = /usr/X11R6/lib
# includes and libs
-INCS = -I. -I/usr/include -I/usr/include/freetype2 -I${X11INC}
-LIBS = -L/usr/lib -lc -lm -L${X11LIB} -lXft -lfontconfig -lX11
+INCS = -I/usr/include/freetype2 -I${X11INC}
+LIBS = -lm -L${X11LIB} -lXft -lfontconfig -lX11
# OpenBSD (uncomment)
-#INCS = -I. -I${X11INC} -I${X11INC}/freetype2
+#INCS = -I${X11INC} -I${X11INC}/freetype2
# FreeBSD (uncomment)
-#INCS = -I. -I/usr/local/include -I/usr/local/include/freetype2 -I${X11INC}
-#LIBS = -L/usr/local/lib -lc -lm -L${X11LIB} -lXft -lfontconfig -lX11
+#INCS = -I/usr/local/include/freetype2 -I${X11INC}
+#LIBS = -lm -L${X11LIB} -lXft -lfontconfig -lX11
# flags
CPPFLAGS = -DVERSION=\"${VERSION}\" -D_XOPEN_SOURCE=600
-CFLAGS += -g -std=c99 -pedantic -Wall ${INCS} ${CPPFLAGS}
-LDFLAGS += -g ${LIBS}
-#CFLAGS += -std=c99 -pedantic -Wall -Os ${INCS} ${CPPFLAGS}
-#LDFLAGS += ${LIBS}
+CFLAGS = -std=c99 -pedantic -Wall -Wstrict-prototypes -Wold-style-definition
-Os ${INCS} ${CPPFLAGS}
+LDFLAGS = ${LIBS}
# compiler and linker
-CC ?= cc
+CC = cc
--
2.36.1
Re: [hackers] [sent] [PATCH] treewide: Improve compliance with our coding style
On Sat, Jun 25, 2022 at 11:38:20PM +0600, NRK wrote:
> On Sat, Jun 25, 2022 at 05:25:54PM +0000, Tom Schwindl wrote:
> > -static void run();
> > -static void usage();
> > -static void xdraw();
> > -static void xhints();
> > -static void xinit();
> > -static void xloadfonts();
> > +static void run(void);
> > +static void usage(void);
> > +static void xdraw(void);
> > +static void xhints(void);
> > +static void xinit(void);
> > +static void xloadfonts(void);
>
> Functions with unspecified arguments is legacy cruft and obsolete since
> C99. I'd perhaps go one step further and add `-Wstrict-prototypes` and
> `-Wold-style-definition` to the list of default warnings in the
> Makefile.
The Makefile and config.mk need to be reworked. Including those options sounds
good.
Some unnecessary extensions are used, too, and `-g` is part of the default flags
which isn't really appropriate.
> > - for (j = 0; j < LEN(fontfallbacks); j++)
> > + for (j = 0; j < LEN(fontfallbacks); j++) {
> > if (fstrs[j])
> > free(fstrs[j]);
> > + }
>
> free on NULL is defined to be no-op. The check could be dropped.
Yes, there are a few occurences of this in the code and removing them should be
fine.
However, I think all those changes should be submitted on their own.
I'll be happy to do that tomorrow.
--
Best Regards,
Tom Schwindl
[hackers] [sent] [PATCH] treewide: Improve compliance with our coding style
- Add `void` as parameter for functions which do not take any Arguments
- Use a block if the body of a loop consists of an `if` statement
- A few changes such as moving a curley bracket
---
diff --git a/drw.c b/drw.c
index c1582e746cc5..84f87b972384 100644
--- a/drw.c
+++ b/drw.c
@@ -19,9 +19,10 @@ static const long utfmax[UTF_SIZ + 1] = {0x10, 0x7F,
0x7FF, 0x, 0x10
static long
utf8decodebyte(const char c, size_t *i)
{
- for (*i = 0; *i < (UTF_SIZ + 1); ++(*i))
+ for (*i = 0; *i < (UTF_SIZ + 1); ++(*i)) {
if (((unsigned char)c & utfmask[*i]) == utfbyte[*i])
return (unsigned char)c & ~utfmask[*i];
+ }
return 0;
}
diff --git a/sent.c b/sent.c
index 9534fcaf97b9..a27b704cd5ce 100644
--- a/sent.c
+++ b/sent.c
@@ -99,12 +99,12 @@ static void load(FILE *fp);
static void advance(const Arg *arg);
static void quit(const Arg *arg);
static void resize(int width, int height);
-static void run();
-static void usage();
-static void xdraw();
-static void xhints();
-static void xinit();
-static void xloadfonts();
+static void run(void);
+static void usage(void);
+static void xdraw(void);
+static void xhints(void);
+static void xinit(void);
+static void xloadfonts(void);
static void bpress(XEvent *);
static void cmessage(XEvent *);
@@ -116,12 +116,12 @@ static void configure(XEvent *);
#include "config.h"
/* Globals */
-static const char *fname = NULL;
-static Slide *slides = NULL;
-static int idx = 0;
-static int slidecount = 0;
+static const char *fname;
+static Slide *slides;
+static int idx;
+static int slidecount;
static XWindow xw;
-static Drw *d = NULL;
+static Drw *d;
static Clr *sc;
static Fnt *fonts[NUMFONTSCALES];
static int running = 1;
@@ -326,9 +326,10 @@ getfontsize(Slide *s, unsigned int *width, unsigned int
*height)
float lfac = linespacing * (s->linecount - 1) + 1;
/* fit height */
- for (j = NUMFONTSCALES - 1; j >= 0; j--)
+ for (j = NUMFONTSCALES - 1; j >= 0; j--) {
if (fonts[j]->h * lfac <= xw.uh)
break;
+ }
LIMIT(j, 0, NUMFONTSCALES - 1);
drw_setfontset(d, fonts[j]);
@@ -381,7 +382,7 @@ cleanup(int slidesonly)
void
reload(const Arg *arg)
{
- FILE *fp = NULL;
+ FILE *fp;
unsigned int i;
if (!fname) {
@@ -414,15 +415,17 @@ load(FILE *fp)
/* read each line from fp and add it to the item list */
while (1) {
/* eat consecutive empty lines */
- while ((p = fgets(buf, sizeof(buf), fp)))
+ while ((p = fgets(buf, sizeof(buf), fp))) {
if (strcmp(buf, "\n") != 0 && buf[0] != '#')
break;
+ }
if (!p)
break;
- if ((slidecount+1) * sizeof(*slides) >= size)
+ if ((slidecount+1) * sizeof(*slides) >= size) {
if (!(slides = realloc(slides, (size += BUFSIZ
die("sent: Unable to reallocate %u bytes:",
size);
+ }
/* read one slide */
maxlines = 0;
@@ -496,7 +499,7 @@ resize(int width, int height)
}
void
-run()
+run(void)
{
XEvent ev;
@@ -518,7 +521,7 @@ run()
}
void
-xdraw()
+xdraw(void)
{
unsigned int height, width, i;
Image *im = slides[idx].img;
@@ -528,15 +531,11 @@ xdraw()
if (!im) {
drw_rect(d, 0, 0, xw.w, xw.h, 1, 1);
- for (i = 0; i < slides[idx].linecount; i++)
- drw_text(d,
-(xw.w - width) / 2,
+ for (i = 0; i < slides[idx].linecount; i++) {
+ drw_text(d, (xw.w - width) / 2,
(xw.h - height) / 2 + i * linespacing *
d->fonts->h,
-width,
-d->fonts->h,
-0,
-slides[idx].lines[i],
-0);
+width, d->fonts->h, 0, slides[idx].lines[i],
0);
+ }
drw_map(d, xw.win, 0, 0, xw.w, xw.h);
} else {
if (!(im->state & SCALED))
@@ -546,7 +545,7 @@ xdraw()
}
void
-xhints()
+xhints(void)
{
XClassHint class = {.res_name = "sent", .res_class = "presenter"};
XWMHints wm = {.flags = InputHint, .input = True};
@@ -564,7 +563,7 @@ xhints()
}
void
-xinit()
+xinit(void)
{
XTextProperty prop;
unsigned int i;
@@ -608,14 +607,13 @@ xinit()
}
void
-xloadfonts()
+xloadfonts(void)
{
int i, j;
char *fstrs[LEN(fontfallbacks)];
- for (j = 0; j < LEN(fontfallbacks); j++) {
+ for (j = 0; j < LEN(fontfallbacks); j++)
fstrs[j] = ecalloc(1, MAXFONTSTRLEN);
- }
for (i = 0; i <
[hackers] [lsw] [PATCH] Makefile: Add POSIX target
Since no extensions are used in the Makefile, add the .POSIX target. --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index b37fa6412ee5..7d7e00dfb370 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,6 @@ # lsw - list window names # See LICENSE file for copyright and license details. +.POSIX: include config.mk -- 2.36.1
