Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tint2: 17.0.2 -> 17.1.3, switch to fork, fix compatibility with glib >= 2.76 #244547

Merged
merged 1 commit into from
Jul 21, 2023

Conversation

SFrijters
Copy link
Member

Description of changes

For a while now tint2 has been crashing for me when I start Steam.
Looking for others with similar problems I found some relevant links

https://bbs.archlinux.org/viewtopic.php?id=284293
https://patchespromptly.com/glib2/
https://gitlab.com/nick87720z/tint2/-/issues/4

Long story short: the original tint2 is no longer maintained, a fork exists (which is not maintained very regularly either), but that fork plus a recent patch fixes the issues for me.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.11 Release Notes (or backporting 23.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@SFrijters SFrijters requested a review from romildo July 20, 2023 19:07
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Jul 20, 2023
@SFrijters
Copy link
Member Author

Result of nixpkgs-review pr 244547 run on x86_64-linux 1

1 package built:
  • tint2

@romildo romildo merged commit 9b23a95 into NixOS:master Jul 21, 2023
@SFrijters SFrijters deleted the tint2-17.1.3 branch July 22, 2023 08:36
@Shados
Copy link
Member

Shados commented Aug 21, 2023

Hmmm. This version itself segfaults on startup for me. Using the o9000 version with the patch from here works fine for me, I'll see if I can git bisect the issue later...

@ben0x539
Copy link
Contributor

ben0x539 commented Dec 5, 2023

I also have been getting segfaults on startup with this version, but the previous version before this PR (old repo, no glib.patch) works. I tracked it down to my config file not having a panel_items key, which the old version copes with better but the current one somehow doesn't. So amending my config is a workaround for me, I guess, I get a bunch of errors still but seems to mostly work:

tint2: got 5 X errors
tint2: BadValue (integer parameter out of range for operation)
tint2: BadDrawable (invalid Pixmap or Window parameter)
tint2: BadValue (integer parameter out of range for operation)
tint2: BadDrawable (invalid Pixmap or Window parameter)
tint2: BadDrawable (invalid Pixmap or Window parameter)
tint2: got 2 X errors
tint2: BadPixmap (invalid Pixmap parameter)
tint2: BadPixmap (invalid Pixmap parameter)

Some details on the panel_items thing:

I crash here:
(gdb) bt
#0  0x00007ffff70e529c in __strlen_evex () from /nix/store/ibp4camsx1mlllwzh32yyqcq2r2xsy1a-glibc-2.37-8/lib/libc.so.6
#1  0x0000000000436f1b in init_separator () at /build/source/src/separator/separator.c:52
#2  0x000000000041d4e8 in init_panel () at /build/source/src/panel.c:202
#3  0x0000000000421e79 in init (argc=argc@entry=1, argv=argv@entry=0x7fffffffba18) at /build/source/src/init.c:322
#4  0x0000000000420db3 in tint2 (argc=argc@entry=1, argv=argv@entry=0x7fffffffba18, restart=restart@entry=0x7fffffffb8e4) at /build/source/src/main.c:817
#5  0x0000000000420f35 in main (argc=1, argv=0x7fffffffba18) at /build/source/src/main.c:874

which is a macro for some sort of loop over a global char *panel_items_order;:

    GList *to_remove = panel_config.separator_list;
    for_panel_items_order (&& to_remove)
        if (panel_items_order[k] == ':')
            to_remove = to_remove->next;
#define for_panel_items_order(...)                                                       \
for (int items_n = strlen(panel_items_order), k = 0; k < items_n __VA_ARGS__; k++)

Before the crash, tint2 logs

tint2: panel items: (null)

The old version that doesn't crash logs:

tint2: panel items: TSCB

which is from panel.c:190,

    fprintf(stderr, "tint2: panel items: %s\n", panel_items_order);

where panel_items_order is initialized in config.c:489

    case key_panel_items:
        new_config_file = TRUE;
        free_and_null(panel_items_order);
        panel_items_order = strdup(value);

So it looks like it's unset when the config doesn't specify panels, and the old version just defaults to

tint2: panel items: TSCB

via a fallback or a bunch of other options implicitly appending some letter or other to that string, but in the current version that, uh, doesn't work anymore, apparently because it's now done via a macro that only appends to a local copy of the variable and doesn't write it back to the global one?

#define _STR_EXTEND(s, L) \
int L; do{                \
    L = strlen(s);        \
    s = realloc(s, L+2);  \
}while(0)

// NOTE: c is 1-char string (null-terminated),
// not a char itself

// also, parts of string could be copied faster using wider data type (up to word)
#define STR_APPEND_CH(str, c)       \
do{ char *s=(str);                  \
    if (s) {                        \
        _STR_EXTEND(s, L);          \
        s[L]=(c)[0], s[L+1]=(c)[1]; \
    } else                          \
        s = strdup(c);              \
}while(0)

#define STR_PREPEND_CH(str, c) \
do{ char *s = (str);           \
    if (s) {                   \
        _STR_EXTEND(s, L);     \
        memmove (s+1, s, L+1); \
        s[0] = (c)[0];         \
    } else                     \
        s = strdup(c);         \
}while(0)

Edit: There's a fix for this at https://gitlab.com/nick87720z/tint2/-/merge_requests/2, but I don't know how alive that repo is now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants