Re: [hackers] [st][PATCH 00/24] odg patches - fix warnings/errors, plug leaks, tidying up
Hi, > What's the point of all this if you are treating others who try to > participate this way ? > I have never seen such lack of tact. Yes, I am pretty sure that you didn't read any of the rants of Linus or Theo. > This sucks. This is suckless, no suckmore.
Re: [hackers] [st][PATCH 00/24] odg patches - fix warnings/errors, plug leaks, tidying up
On Wed, Jan 23, 2019 at 03:54:35AM +0100, kais euchi wrote: > Greetings ! > What's the point of all this if you are treating others who try to > participate this way ? > I have never seen such lack of tact. > This sucks. Hi, You must be new to the list. Best regards, David
Re: [hackers] [st][PATCH 00/24] odg patches - fix warnings/errors, plug leaks, tidying up
Greetings ! What's the point of all this if you are treating others who try to participate this way ? I have never seen such lack of tact. This sucks. On 19-01-22 08:15:20, k...@shike2.com wrote: > Hi, > > > > These patches fix all errors and warnings I found when building with lots of > > compiler warning flags enabled, and running tools like cppcheck and > > valgrind. > > There's lots of type and const correctness fixes, conflicting variable > > names, > > a couple of memory leaks, and stylistic stuff like reducing variable scope. > > Also avoid linking with libm and make building on OpenBSD a bit more > > straightforward. > > First point, why do you think we care about what your linter says? If it > gives false positives send patches to the developers of your linter. > > > Oliver Galvin (24): > > add gitignore > > Why do you think we want a gitignore?. Suckless projects don't use > .gitignore files. If you want to ignore files you can do it locally > using .git/info/exclude > > > avoid unnecessarily checking if an unsigned variable is < 0, fixes > > cppcheck warnings > > I don't care about cppcheck. And you modified the macro ISCONTROL() > in a way that hides the difference beteween C0 control codes and C1 > control codes. It is true that the code doesn't use this difference, > but it is additional documentation in the code. > > > Reduce variable scope where possible, fix cppcheck style warnings > > I don't care about cppcheck. Suckless projects always declare the > variable at the beginning of the function. > > > use const where possible, avoid discarding const, fixes errors from > > -Wdiscarded-qualifiers > > I don't care about cppcheck. > > > avoid warnings from -Wunused-parameter > > I don't care about cppcheck. > > > fix warnings from -Wimplicit-fallthrough > > I don't care about cppcheck. And you are modifying the behaviour of st. > How do you think that you can put random breaks and you are not going to > break the code?!?!?!?!?!?!? > > > fix warning from -Wmaybe-uninitialized > > I don't care about cppcheck. > > > fix warnings from -Wsign-compare and -Wtype-limits. also make sure we > > use size_t for len variables > > I don't care about cppcheck. > > > avoid redundant declaration and old-style function definition > > -extern char *argv0; > +static char *argv0; > > Do you understand what you are doing there?!?!??!!?. You are creating > a different variable in every file including arg.h!!!. > > > fix -Wshadow warnings, due to variable names conflicting with global > > variables. also we don't need to pass global variables to xinit() > > I don't care about cppcheck. > > > rename variable to fix cppcheck shadow warning, due to variable name > > conflicting with function > > I don't care about cppcheck. > > > fix remaining cppcheck warnings: reduce scope of some variables, and > > avoid compiling selcheck_ when it's not used, by adding a new > > setting in config.h > > I don't care about cppcheck. Why do you think this ifdef is a good thing??? > Because we move from 45KB to 44.9KB? > > > clean up two warnings from clang about initialisation and sign > > comparison > > I don't care about cppcheck. And now, I don't care about clang. Send > patches to clang to remove false positives. > > > now st can build without errors/warnings with -Wall -Wextra -Wpedantic > > enabled, enable them by default > > I don't care about cppcheck and I don't care about your warnings. > > > update config.def.h with necessary changes after my previous commits. > > uses const everywhere and adds the SELCLEAR option > > Ok, at this point I can confirm that you don't have too much experience > in programming, it is obvious. A free advise, don't hurt yourself and > don't use const. > > > avoid leaking memory when xrealloc/xstrdup fail, by freeing memory > > before die() > > I don't care about cppcheck and at this moment I don't care about valgrind. > There isn't any memory leak, all the memory of the process is freed at > the end of the process. > > > use EXIT_SUCCESS/FAILURE > > Please, don't submit style patches. St is a portable posix program, and in > the posix enviroment the exit status is properly defined and the use of 0 > and 1 is correct. > > > use compiler attribute to check die() parameters, and fix relevant > > warning from clang > > Over my dead body. Learn to use C, please don't use GNU extensions. At this > moment st can be compiled with any c99 compliant compiler: gcc, clang, pcc, > lcc, tcc ... > > > improve type and const correctness by using more correct types. fixes > > a bunch of -Wcast-qual warnings > > Your patch is not correct, rejected. > > > use EXIT_FAILURE/SUCCESS on exit > > At least you should learn to squash commits before sending them to any > open source project. That's a pity that the mail history cannot be > rewritten. Any recruiter that will sea
[hackers] [st][PATCH] Fix crash on IME restart
Register XNDestroyCallback on IME to reinstatiate XIM.
This does not solve:
- IME not usable if st started before IME
- st goes blank with two st in a dwm tag after restarting IME, might be
dwm bug as it is happens to other terminal as well
---
x.c | 61 +
1 file changed, 45 insertions(+), 16 deletions(-)
diff --git a/x.c b/x.c
index 0422421..e5ae6a2 100644
--- a/x.c
+++ b/x.c
@@ -139,6 +139,9 @@ static void xdrawglyphfontspecs(const XftGlyphFontSpec *,
Glyph, int, int, int);
static void xdrawglyph(Glyph, int, int);
static void xclear(int, int, int, int);
static int xgeommasktogravity(int);
+static void ximopen(Display *);
+static void ximinstantiate(Display *, XPointer, XPointer);
+static void ximdestroy(XIM, XPointer, XPointer);
static void xinit(int, int);
static void cresize(int, int);
static void xresize(int, int);
@@ -996,6 +999,47 @@ xunloadfonts(void)
xunloadfont(&dc.ibfont);
}
+void
+ximopen(Display *dpy)
+{
+ XIMCallback destroy = { .client_data = NULL, .callback = ximdestroy };
+
+ if ((xw.xim = XOpenIM(xw.dpy, NULL, NULL, NULL)) == NULL) {
+ XSetLocaleModifiers("@im=local");
+ if ((xw.xim = XOpenIM(xw.dpy, NULL, NULL, NULL)) == NULL) {
+ XSetLocaleModifiers("@im=");
+ if ((xw.xim = XOpenIM(xw.dpy,
+ NULL, NULL, NULL)) == NULL) {
+ die("XOpenIM failed. Could not open input"
+ " device.\n");
+ }
+ }
+ }
+ if (XSetIMValues(xw.xim, XNDestroyCallback, &destroy, NULL) != NULL)
+ die("XSetIMValues failed. Could not set input method value.\n");
+ xw.xic = XCreateIC(xw.xim, XNInputStyle,
+ XIMPreeditNothing | XIMStatusNothing,
+ XNClientWindow, xw.win, XNFocusWindow, xw.win,
NULL);
+ if (xw.xic == NULL)
+ die("XCreateIC failed. Could not obtain input method.\n");
+}
+
+void
+ximinstantiate(Display *dpy, XPointer client, XPointer call)
+{
+ ximopen(dpy);
+ XUnregisterIMInstantiateCallback(xw.dpy, NULL, NULL, NULL,
+ ximinstantiate, NULL);
+}
+
+void
+ximdestroy(XIM xim, XPointer client, XPointer call)
+{
+ xw.xim = NULL;
+ XRegisterIMInstantiateCallback(xw.dpy, NULL, NULL, NULL,
+ ximinstantiate, NULL);
+}
+
void
xinit(int cols, int rows)
{
@@ -1061,22 +1105,7 @@ xinit(int cols, int rows)
xw.draw = XftDrawCreate(xw.dpy, xw.buf, xw.vis, xw.cmap);
/* input methods */
- if ((xw.xim = XOpenIM(xw.dpy, NULL, NULL, NULL)) == NULL) {
- XSetLocaleModifiers("@im=local");
- if ((xw.xim = XOpenIM(xw.dpy, NULL, NULL, NULL)) == NULL) {
- XSetLocaleModifiers("@im=");
- if ((xw.xim = XOpenIM(xw.dpy,
- NULL, NULL, NULL)) == NULL) {
- die("XOpenIM failed. Could not open input"
- " device.\n");
- }
- }
- }
- xw.xic = XCreateIC(xw.xim, XNInputStyle, XIMPreeditNothing
- | XIMStatusNothing, XNClientWindow,
xw.win,
- XNFocusWindow, xw.win, NULL);
- if (xw.xic == NULL)
- die("XCreateIC failed. Could not obtain input method.\n");
+ ximopen(xw.dpy);
/* white cursor, black outline */
cursor = XCreateFontCursor(xw.dpy, mouseshape);
--
2.20.1
Re: [hackers] [st][PATCH 20/24] use EXIT_FAILURE/SUCCESS on exit
On Mon, 21 Jan 2019 23:56:35 +
Oliver Galvin wrote:
Dear Galvin,
I would object to such a change. We don't use EXIT_*-defines for a
reason.
With best regards
Laslo
> ---
> st.c | 4 ++--
> x.c | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/st.c b/st.c
> index 5a8cdcc..4b5e725 100644
> --- a/st.c
> +++ b/st.c
> @@ -718,7 +718,7 @@ execsh(char *cmd, char *const *args)
> signal(SIGALRM, SIG_DFL);
>
> execvp(prog, args);
> - _exit(1);
> + _exit(EXIT_FAILURE);
> }
>
> void
> @@ -735,7 +735,7 @@ sigchld(UNUSED int a)
>
> if (!WIFEXITED(stat) || WEXITSTATUS(stat))
> die("child finished with error '%d'\n", stat);
> - exit(0);
> + exit(EXIT_SUCCESS);
> }
>
> void
> diff --git a/x.c b/x.c
> index 9e9bd3e..bd55699 100644
> --- a/x.c
> +++ b/x.c
> @@ -1751,7 +1751,7 @@ cmessage(XEvent *e)
> }
> } else if (e->xclient.data.l[0] >= 0 && (Atom)
> (e->xclient.data.l[0]) == xw.wmdeletewin) { ttyhangup();
> - exit(0);
> + exit(EXIT_SUCCESS);
> }
> }
>
> --
> 2.20.1
--
Laslo Hunhold
pgp7PrP6soz3S.pgp
Description: PGP signature
Re: [hackers] [st][PATCH 14/24] now st can build without errors/warnings with -Wall -Wextra -Wpedantic enabled, enable them by default
Hello again, > +WARN = -Wall -Wextra -Wpedantic > CPPFLAGS = -DVERSION=\"$(VERSION)\" -D_XOPEN_SOURCE=600 > -STCFLAGS = $(INCS) $(CPPFLAGS) $(CFLAGS) > -STLDFLAGS = $(LIBS) $(LDFLAGS) > +STCFLAGS = $(INCS) $(CPPFLAGS) $(CFLAGS) $(WARN) > +STLDFLAGS = $(LIBS) $(LDFLAGS) $(WARN) I think this is exactly what's wrong here. Those warning of for development purposes, not software publication. That's ok, when you're writing code, to possibly turn those on, read what they say, and then decide if they're correct or not (everybody can make a mistake). Those warnings are not a golf game, where you'd have to modify code until they don't show up anymore. Again, they're just here to show something suspect in the (blind) eye of the compiler, but ultimately you're the one in control there and (should) know what you're doing. When in doubt, re-open the standard(s) for answers. When you didn't write the original code, you have to understand what the original author wanted to do, that's easier when he wrote it in a sane way. The only acceptable GNU warning flag here maybe be -w.
Re: [hackers] [st][PATCH 00/24] odg patches - fix warnings/errors, plug leaks, tidying up
Hi Oliver, > > These patches fix all errors and warnings I found when building with lots of > > compiler warning flags enabled, and running tools like cppcheck and > > valgrind. > > There's lots of type and const correctness fixes, conflicting variable > > names, > > a couple of memory leaks, and stylistic stuff like reducing variable scope. > > Also avoid linking with libm and make building on OpenBSD a bit more > > straightforward. > > First point, why do you think we care about what your linter says? If it > gives false positives send patches to the developers of your linter. Thanks for your interest, but yeah, obviously you didn't take a look at the project before diving head on into bikeshedding patches (mostly). But that's ok, everybody has to learn, you are. This doesn't mean you have to stop there, just get back on it and avoid those kind of “patching what compiler warnings say” while taking in mind what the project you're patching is about and how it's written (and more precisely why).
Re: [hackers] [st][PATCH 00/24] odg patches - fix warnings/errors, plug leaks, tidying up
Hi, > These patches fix all errors and warnings I found when building with lots of > compiler warning flags enabled, and running tools like cppcheck and valgrind. > There's lots of type and const correctness fixes, conflicting variable names, > a couple of memory leaks, and stylistic stuff like reducing variable scope. > Also avoid linking with libm and make building on OpenBSD a bit more > straightforward. First point, why do you think we care about what your linter says? If it gives false positives send patches to the developers of your linter. > Oliver Galvin (24): > add gitignore Why do you think we want a gitignore?. Suckless projects don't use .gitignore files. If you want to ignore files you can do it locally using .git/info/exclude > avoid unnecessarily checking if an unsigned variable is < 0, fixes > cppcheck warnings I don't care about cppcheck. And you modified the macro ISCONTROL() in a way that hides the difference beteween C0 control codes and C1 control codes. It is true that the code doesn't use this difference, but it is additional documentation in the code. > Reduce variable scope where possible, fix cppcheck style warnings I don't care about cppcheck. Suckless projects always declare the variable at the beginning of the function. > use const where possible, avoid discarding const, fixes errors from > -Wdiscarded-qualifiers I don't care about cppcheck. > avoid warnings from -Wunused-parameter I don't care about cppcheck. > fix warnings from -Wimplicit-fallthrough I don't care about cppcheck. And you are modifying the behaviour of st. How do you think that you can put random breaks and you are not going to break the code?!?!?!?!?!?!? > fix warning from -Wmaybe-uninitialized I don't care about cppcheck. > fix warnings from -Wsign-compare and -Wtype-limits. also make sure we > use size_t for len variables I don't care about cppcheck. > avoid redundant declaration and old-style function definition -extern char *argv0; +static char *argv0; Do you understand what you are doing there?!?!??!!?. You are creating a different variable in every file including arg.h!!!. > fix -Wshadow warnings, due to variable names conflicting with global > variables. also we don't need to pass global variables to xinit() I don't care about cppcheck. > rename variable to fix cppcheck shadow warning, due to variable name > conflicting with function I don't care about cppcheck. > fix remaining cppcheck warnings: reduce scope of some variables, and > avoid compiling selcheck_ when it's not used, by adding a new > setting in config.h I don't care about cppcheck. Why do you think this ifdef is a good thing??? Because we move from 45KB to 44.9KB? > clean up two warnings from clang about initialisation and sign > comparison I don't care about cppcheck. And now, I don't care about clang. Send patches to clang to remove false positives. > now st can build without errors/warnings with -Wall -Wextra -Wpedantic > enabled, enable them by default I don't care about cppcheck and I don't care about your warnings. > update config.def.h with necessary changes after my previous commits. > uses const everywhere and adds the SELCLEAR option Ok, at this point I can confirm that you don't have too much experience in programming, it is obvious. A free advise, don't hurt yourself and don't use const. > avoid leaking memory when xrealloc/xstrdup fail, by freeing memory > before die() I don't care about cppcheck and at this moment I don't care about valgrind. There isn't any memory leak, all the memory of the process is freed at the end of the process. > use EXIT_SUCCESS/FAILURE Please, don't submit style patches. St is a portable posix program, and in the posix enviroment the exit status is properly defined and the use of 0 and 1 is correct. > use compiler attribute to check die() parameters, and fix relevant > warning from clang Over my dead body. Learn to use C, please don't use GNU extensions. At this moment st can be compiled with any c99 compliant compiler: gcc, clang, pcc, lcc, tcc ... > improve type and const correctness by using more correct types. fixes > a bunch of -Wcast-qual warnings Your patch is not correct, rejected. > use EXIT_FAILURE/SUCCESS on exit At least you should learn to squash commits before sending them to any open source project. That's a pity that the mail history cannot be rewritten. Any recruiter that will search for your name in the future will discover this shame of patchset. Second free advise, before doing things try to learn from more experienced guys. > avoid unnecessarily linking to the math library, and detect openbsd > automatically in makefile Over my dead body!!!, what the fuck are you thinking???. Only the part of removing -m is good. You should learn to use POSIX Makefiles. At this moment st can be compiled using any POSIX Make: GNU make, BSD m
