Opened 9 years ago

Closed 8 years ago

#11958 closed defect (fixed)

Get Info on contact doesn't work with Gadu-Gadu protocol.

Reported by: diskape Owned by: bartosz
Milestone: 2.8.0 Component: Gadu-Gadu
Version: 2.7.0 Keywords:
Cc: rg_gapa, 6thsurvivor, fabek

Description

It's stuck on "Information: Retrieving..."

Attachments (2)

search_contact_fix.diff (1.8 KB) - added by matekm 8 years ago.
FIx for broken gg contact search on Windows build
gg_pragma_pack.diff (822 bytes) - added by datallah 8 years ago.

Download all attachments as: .zip

Change History (47)

comment:1 follow-up: Changed 9 years ago by darkrain42

  • Status changed from new to pending

Please follow the instructions to get a debug log and attach it to this ticket.

comment:2 follow-up: Changed 9 years ago by kkszysiu

Strange thing. Windows or UNIX? I'm using it on Linux all time and it works perfectly :>

comment:3 in reply to: ↑ 2 Changed 9 years ago by diskape

  • Status changed from pending to new

Replying to kkszysiu:

Strange thing. Windows or UNIX? I'm using it on Linux all time and it works perfectly :>

I'm on Windows7 x64 and I have some privacy concerns about sending my debug.log file. This bug is very easy to replicate tho so maybe someone could post their logs. Best I could do is to open a ticket. I also checked different account on a different Win7 machine and it's the same thing.

comment:4 Changed 9 years ago by Robby

Ticket #12122 has been marked as a duplicate of this ticket.

comment:5 in reply to: ↑ 1 Changed 9 years ago by rocketman

Replying to darkrain42:

Please follow the instructions to get a debug log and attach it to this ticket.

Here's log after trying to get user info from buddy list.

Windows 7 x32
Pidgin 2.7.1(bug appeared on Pidgin 2.7.0)
3rd party plugins disabled

15:19:24) gtkappbar: wnd_poschanging
(15:19:24) gtkappbar: wnd_poschanging
(15:19:26) gtkappbar: wnd_poschanging
(15:19:26) gtkappbar: wnd_poschanging
(15:19:26) gg: It's time to perform a search...
(15:19:26) gg: ** gg_pubdir50_new(3);
(15:19:26) gg:     uin: 7360142
(15:19:26) gg: ** gg_pubdir50_add_n(05D43CB0, 0, "FmNumber", "7360142");
(15:19:26) gg: offset: 0
(15:19:26) gg: ** gg_pubdir50_add_n(05D43CB0, 0, "fmstart", "0");
(15:19:26) gg: ** gg_pubdir50(027EA1C8, 05D43CB0);
(15:19:26) gg: ** gg_send_packet(027EA1C8, 0x14, ...);
(15:19:26) gg: // gg_send_packet() partial write(), 40 sent, 0 left, 0 total left
(15:19:26) gg: search sequence number: 1276435166
(15:19:26) gg: ggp_get_info(): Added seq 1276435166(15:19:26) gtkappbar: wnd_poschanging
(15:19:26) gg: ** gg_watch_fd(027EA1C8);
(15:19:26) gg: // gg_watch_fd() GG_STATE_CONNECTED
(15:19:26) gg: ** gg_watch_fd_connected(027EA1C8, 05D18DD8);
(15:19:26) gg: ** gg_recv_packet(027EA1C8);
(15:19:26) gg: // gg_recv_packet() header recv(892,0022E510,8) = 8
(15:19:26) gg: // gg_recv_packet() body recv(892,049A0C00,80) = 80
(15:19:26) gg: // gg_watch_fd_connected() received pubdir/search reply
(15:19:26) gg: ** gg_pubdir50_handle_reply_sess(027EA1C8, 05D18DD8, 049A0C00, 80);
(15:19:26) gg: ** gg_pubdir50_new(5);
(15:19:26) gg: ** gg_pubdir50_add_n(05DA9090, 0, "FmNumber", "7360142");
(15:19:26) gg: ** gg_pubdir50_add_n(05DA9090, 0, "FmStatus", "3");
(15:19:26) gg: ** gg_pubdir50_add_n(05DA9090, 0, "city", "Krakow");
(15:19:26) gg: ** gg_pubdir50_add_n(05DA9090, 0, "firstname", "Mateusz");
(15:19:26) gg: ** gg_pubdir50_add_n(05DA9090, 0, "nickname", "mati304");
(15:19:26) gg: ggp_pubdir_reply_handler(): seq 1315784414 --> form 00000000(15:19:26) g_log: ggp_pubdir_reply_handler: assertion `form != NULL' failed
(15:19:26) gg: ** gg_event_free(05D18DD8);

comment:6 Changed 9 years ago by rg_gapa

Here is my debug log, i have the same bug:

(11:56:46) gg: It's time to perform a search...
(11:56:46) gg: ** gg_pubdir50_new(3);
(11:56:46) gg:     uin: 1580434
(11:56:46) gg: ** gg_pubdir50_add_n(089B0880, 0, "FmNumber", "1580434");
(11:56:46) gg: offset: 0
(11:56:46) gg: ** gg_pubdir50_add_n(089B0880, 0, "fmstart", "0");
(11:56:46) gg: ** gg_pubdir50(062F4858, 089B0880);
(11:56:46) gg: ** gg_send_packet(062F4858, 0x14, ...);
(11:56:46) gg: // gg_send_packet() partial write(), 40 sent, 0 left, 0 total left
(11:56:46) gg: search sequence number: 1278237406
(11:56:46) gg: ggp_get_info(): Added seq 1278237406(11:56:46) gg: ** gg_watch_fd(062F4858);
(11:56:46) gg: // gg_watch_fd() GG_STATE_CONNECTED
(11:56:46) gg: ** gg_watch_fd_connected(062F4858, 05B79A18);
(11:56:46) gg: ** gg_recv_packet(062F4858);
(11:56:46) gg: // gg_recv_packet() header recv(2920,0022E510,8) = 8
(11:56:46) gg: // gg_recv_packet() body recv(2920,062FDF50,68) = 68
(11:56:46) gg: // gg_watch_fd_connected() received pubdir/search reply
(11:56:46) gg: ** gg_pubdir50_handle_reply_sess(062F4858, 05B79A18, 062FDF50, 68);
(11:56:46) gg: ** gg_pubdir50_new(5);
(11:56:46) gg: ** gg_pubdir50_add_n(08BFCA48, 0, "FmNumber", "1580434");
(11:56:46) gg: ** gg_pubdir50_add_n(08BFCA48, 0, "FmStatus", "2");
(11:56:46) gg: ** gg_pubdir50_add_n(08BFCA48, 0, "firstname", "Mateusz");
(11:56:46) gg: ** gg_pubdir50_add_n(08BFCA48, 0, "nickname", "1580434");
(11:56:46) gg: ggp_pubdir_reply_handler(): seq 1315784414 --> form 00000000(11:56:46) g_log: ggp_pubdir_reply_handler: assertion `form != NULL' failed
(11:56:46) gg: ** gg_event_free(05B79A18);

comment:7 Changed 9 years ago by darkrain42

Ticket #12354 has been marked as a duplicate of this ticket.

comment:8 Changed 9 years ago by rekkanoryo

Ticket #12583 has been marked as a duplicate of this ticket.

comment:9 Changed 9 years ago by Robby

Ticket #12682 has been marked as a duplicate of this ticket.

comment:10 Changed 9 years ago by Robby

Ticket #12685 has been marked as a duplicate of this ticket.

comment:11 Changed 9 years ago by Robby

Ticket #12713 has been marked as a duplicate of this ticket.

comment:12 Changed 9 years ago by datallah

Ticket #12727 has been marked as a duplicate of this ticket.

comment:13 Changed 8 years ago by Robby

Ticket #12782 has been marked as a duplicate of this ticket.

comment:14 Changed 8 years ago by 6thsurvivor

Can we change the ticket owner or change it to patches welcome? I know that breaking a core functionality(obtaining contact/new contact information - without it you only see contacts as a number) for the main IM protocole in Poland is not an critical issue when it works under linux(AFAIK both bartosz and kkszysiu are linux users and the error is only under windows), but for for windows users/multibooters it's a huge regression since 2.6.6 - last build which worked, which didn't support all nice protocol changes addes in 2.7.0 making it semi-incompatibile with the current protocole implementation.

comment:15 Changed 8 years ago by datallah

  • Milestone set to Patches welcome

We certainly could change the owner and the milestone, but that isn't going to get this fixed any faster.

comment:16 Changed 8 years ago by kkszysiu

The main question is why it's not working under Windows because since 2.6.6 theres no changes in code of it. I could look at it in a free time but it will be hard because I don't have Windows+Cygwin installation now.

comment:17 Changed 8 years ago by datallah

What do you mean that there are no code changes since 2.6.6? In 2.7.0, the libgadu was bumped to 1.9.0-rc2. It is entirely possible that this is caused by a bug in libgadu (or the changes made to libgadu by us to work on win32).

comment:18 Changed 8 years ago by Robby

Ticket #12838 has been marked as a duplicate of this ticket.

comment:19 Changed 8 years ago by Robby

Ticket #13132 has been marked as a duplicate of this ticket.

comment:20 Changed 8 years ago by Robby

Ticket #13145 has been marked as a duplicate of this ticket.

comment:21 Changed 8 years ago by Robby

Ticket #13192 has been marked as a duplicate of this ticket.

comment:22 Changed 8 years ago by fabek

When you're going to fix this bug? Many peoples waiting for that. Previous versions of Pidgin did not have this problem.

comment:23 follow-up: Changed 8 years ago by darkrain42

Gadu Gadu support is provided primarily through third-party contributions to the protocol plugin. The only developer who has/had an account is no longer active.

That's why this is set to a milestone of "Patches welcome".

comment:24 in reply to: ↑ 23 Changed 8 years ago by 6thsurvivor

Replying to darkrain42:

Gadu Gadu support is provided primarily through third-party contributions to the protocol plugin. The only developer who has/had an account is no longer active.

That's why this is set to a milestone of "Patches welcome".

Keep in mind, that only windows version is affected.

comment:25 follow-ups: Changed 8 years ago by kkszysiu

Well, I could try to fix that but I don't have access to any Windows OS for now :) Maybe I'll look into that at weekend.

comment:26 in reply to: ↑ 25 Changed 8 years ago by fabek

Replying to kkszysiu:

Well, I could try to fix that but I don't have access to any Windows OS for now :) Maybe I'll look into that at weekend.

You can do it kkszysiu - dasz radę kkrzysiu =)

comment:27 in reply to: ↑ 25 Changed 8 years ago by 6thsurvivor

Replying to kkszysiu:

Well, I could try to fix that but I don't have access to any Windows OS for now :) Maybe I'll look into that at weekend.

Would a rdp/vnc connection to a remote windows machine make any difference?:)

comment:28 Changed 8 years ago by rg_gapa

almost chat in here ;D I hope that this bug will b repaired soon :/ What pity that I can't help at this moment, cause I'm just newbie in programming

comment:29 Changed 8 years ago by buzdygan

I don't know if this will be of any help, but I'm using both Windows and Linux and here are logs from both when trying to get information on a contact:

Linux (working correctly):

Pidgin Debug Log : nie, 16 sty 2011, 10:58:34
(10:58:21) gg: It's time to perform a search...
(10:58:21) gg: ** gg_pubdir50_new(3);
(10:58:21) gg:     uin: 12880226
(10:58:21) gg: ** gg_pubdir50_add_n(0x8909d58, 0, "FmNumber", "12880226");
(10:58:21) gg: offset: 0
(10:58:21) gg: ** gg_pubdir50_add_n(0x8909d58, 0, "fmstart", "0");
(10:58:21) gg: ** gg_pubdir50(0x871ffe0, 0x8909d58);
(10:58:21) gg: ** gg_send_packet(0x871ffe0, 0x14, ...);
(10:58:21) gg: // gg_send_packet() partial write(), 41 sent, 0 left, 0 total left
(10:58:21) gg: search sequence number: 1295171901
(10:58:21) gg: ggp_get_info(): Added seq 1295171901(10:58:22) gg: ** gg_watch_fd(0x871ffe0);
(10:58:22) gg: // gg_watch_fd() GG_STATE_CONNECTED
(10:58:22) gg: ** gg_watch_fd_connected(0x871ffe0, 0x890b6d0);
(10:58:22) gg: ** gg_recv_packet(0x871ffe0);
(10:58:22) gg: // gg_recv_packet() header recv(9,0xbfb03514,8) = 8
(10:58:22) gg: // gg_recv_packet() body recv(9,0x890c6f8,76) = 76
(10:58:22) gg: // gg_watch_fd_connected() received pubdir/search reply
(10:58:22) gg: ** gg_pubdir50_handle_reply_sess(0x871ffe0, 0x890b6d0, 0x890c6f8, 76);
(10:58:22) gg: ** gg_pubdir50_new(5);
(10:58:22) gg: ** gg_pubdir50_add_n(0x890d9a0, 0, "FmNumber", "12880226");
(10:58:22) gg: ** gg_pubdir50_add_n(0x890d9a0, 0, "FmStatus", "1");
(10:58:22) gg: ** gg_pubdir50_add_n(0x890d9a0, 0, "birthyear", "1992");
(10:58:22) gg: ** gg_pubdir50_add_n(0x890d9a0, 0, "city", "slupsk");
(10:58:22) gg: ** gg_pubdir50_add_n(0x890d9a0, 0, "firstname", "Jula");
(10:58:22) gg: ggp_pubdir_reply_handler(): seq 1295171901 --> form 0x8909e80(10:58:22) gg: ** gg_pubdir50_get(0x890d9a0, 0, "FmStatus");
(10:58:22) gg: ggp_status_by_id: 1
(10:58:22) gg: ** gg_pubdir50_get(0x890d9a0, 0, "FmNumber");
(10:58:22) gg: ** gg_pubdir50_get(0x890d9a0, 0, "firstname");
(10:58:22) gg: ** gg_pubdir50_get(0x890d9a0, 0, "nickname");
(10:58:22) gg: ** gg_pubdir50_get(0x890d9a0, 0, "city");
(10:58:22) gg: ** gg_pubdir50_get(0x890d9a0, 0, "birthyear");
(10:58:22) gg: ** gg_event_free(0x890b6d0);

Windows (not working):

Pidgin Debug Log : 2011-01-15 18:37:56
(18:37:35) gg: It's time to perform a search...
(18:37:35) gg: ** gg_pubdir50_new(3);
(18:37:35) gg:     uin: 12880226
(18:37:35) gg: ** gg_pubdir50_add_n(00BB8EE0, 0, "FmNumber", "12880226");
(18:37:35) gg: offset: 0
(18:37:35) gg: ** gg_pubdir50_add_n(00BB8EE0, 0, "fmstart", "0");
(18:37:35) gg: ** gg_pubdir50(00B179E0, 00BB8EE0);
(18:37:35) gg: ** gg_send_packet(00B179E0, 0x14, ...);
(18:37:35) gg: // gg_send_packet() partial write(), 41 sent, 0 left, 0 total left
(18:37:35) gg: search sequence number: 1295113055
(18:37:35) gg: ggp_get_info(): Added seq 1295113055(18:37:35) gg: ** gg_watch_fd(00B179E0);
(18:37:35) gg: // gg_watch_fd() GG_STATE_CONNECTED
(18:37:35) gg: ** gg_watch_fd_connected(00B179E0, 00BBD6C0);
(18:37:35) gg: ** gg_recv_packet(00B179E0);
(18:37:35) gg: // gg_recv_packet() header recv(440,0022E510,8) = 8
(18:37:35) gg: // gg_recv_packet() body recv(440,00BB7A68,76) = 76
(18:37:35) gg: // gg_watch_fd_connected() received pubdir/search reply
(18:37:35) gg: ** gg_pubdir50_handle_reply_sess(00B179E0, 00BBD6C0, 00BB7A68, 76);
(18:37:35) gg: ** gg_pubdir50_new(5);
(18:37:35) gg: ** gg_pubdir50_add_n(00BBB340, 0, "FmNumber", "12880226");
(18:37:35) gg: ** gg_pubdir50_add_n(00BBB340, 0, "FmStatus", "1");
(18:37:35) gg: ** gg_pubdir50_add_n(00BBB340, 0, "birthyear", "1992");
(18:37:35) gg: ** gg_pubdir50_add_n(00BBB340, 0, "city", "slupsk");
(18:37:35) gg: ** gg_pubdir50_add_n(00BBB340, 0, "firstname", "Jula");
(18:37:35) gg: ggp_pubdir_reply_handler(): seq 1315784287 --> form 00000000(18:37:35) g_log: ggp_pubdir_reply_handler: assertion `form != NULL' failed
(18:37:35) gg: ** gg_event_free(00BBD6C0);

I don't understand most of this code, but I can see that on Windows, the information is received from the server, but it's just not displayed in the information window.

comment:30 Changed 8 years ago by Misiur

As we can see this line is source of error.

(18:37:35) gg: ggp_pubdir_reply_handler(): seq 1315784287 --> form 00000000(18:37:35) g_log: ggp_pubdir_reply_handler: assertion `form != NULL' failed

Diffrence between Linux form (variable?)

form 0x8909e80

And Windows form

form 00000000

In file gg.c I've found

form = ggp_search_get(info->searches, seq);

seq is correct value, info (IMHO) is good too, because it's used elsewhere (GGPInfo *info = gc->proto_data;)

What is ggp_search_get?

GGPSearchForm *ggp_search_get(GGPSearches *searches, guint32 seq)
{
	g_return_val_if_fail(searches != NULL, NULL);

	return g_hash_table_lookup(searches, &seq);
}

I'm only PHP programmer, so I can't help, I don't know even how to compile properly lol, but I'd really love to see working status retrieving (especially it's annoying when I need someone's status). I appreciate everyone's work.

Also: http://developer.pidgin.im/ticket/13400 - this means that we need someone from outside to help, or only solution is waiting?

comment:31 Changed 8 years ago by matekm

I looked at this and noticed that in linux:

10:58:21) gg: search sequence number: 1295171901
(10:58:21) gg: ggp_get_info(): Added seq 1295171901(10:58:22) gg: ** gg_watch_fd(0x871ffe0);

(10:58:22) gg: ggp_pubdir_reply_handler(): seq 1295171901 --> form 0x8909e80(10:58:22) gg: ** gg_pubdir50_get(0x890d9a0, 0, "FmStatus");

the seq number is the same when adding to has table and when retrieving form from this hash table. On Windows on the other hand:

(18:37:35) gg: search sequence number: 1295113055
(18:37:35) gg: ggp_get_info(): Added seq 1295113055(18:37:35) gg: ** gg_watch_fd(00B179E0);

(18:37:35) gg: ggp_pubdir_reply_handler(): seq 1315784287 --> form 00000000(18:37:35) g_log: ggp_pubdir_reply_handler: assertion `form != NULL' failed

The seq number used to retrieve form from hash table is different that from the one that was used to add it to the hash table.

I dig little into the code and found that in gg_pubdir50_handle_reply_sess function (pubdir50.cc) res->seq = gg_fix32(r->seq);' - the res->seq is set to wrong number. Still not sure if it is root of the problem or why only windows builds are affected.

comment:32 Changed 8 years ago by matekm

Ok, I found the root of the problem: it's look like there is an issue with struct alignment on Windows.

There are some cast to struct gg_pubdir50_reply * used in gg_pubdir50 (pubdir50.cc):

r = (struct gg_pubdir50_request*) buf;
r->type = req->type;
r->seq = gg_fix32(req->seq);

Because struct gg_pubdir50_reply is defined as packed, sizeof(*r) should be 5 (uint8_t + uint32_t). But it isn't, it is 8.

So, in memory we have: {

0x00000003 /* type */ 0xE4927B4d /* seq number */

}

but then we have

for (i = 0, p = buf + 5; i < req->entries_count; i++) {

and we override seq number with search data.

The same situation is in gg_pubdir50_handle_reply_sess - the same alignment problem.

Because of this we can't correctly retrieve sequence number and can't correctly lookup form in hash table.

I replaced casts to the struct with memcpy and pubdir search started working.

Now, I don't think that this is problem with pidgin/libgadu (I copy the gg implementation from 2.6.6 and the pubdir search wasn't working too). Rather this is cygwing/mingw issue. Any idea what changed in those tools? How can this be fixed?

comment:33 Changed 8 years ago by kkszysiu

Working on that. I'm working on libgadu 1.10.0 as static, SSL support in GG and typing notify too. Hope you will see my work soon in Pidgin main repo :)

comment:34 Changed 8 years ago by matekm

Where are You with this ticket (e.g. how close You are to fix this:))?

comment:35 Changed 8 years ago by kkszysiu

I will install Windows soon (give me a year and a half to prepare everything) then I will start hacking it for Windows :D

comment:36 Changed 8 years ago by matekm

So, it definetly looks like issue with gcc on windows. There is alignment error when using gcc-core-4.4.0 but when I replaced it with gcc-core-3.4.2 and rebuilded the source everythind worked great. Why the 4.4.0 core don't work?

Tommorow I try with the gcc-core-4.5.2 and if there will be problem we should back to 3.4.2 (only gcc-core) on Windows until we resolve the problem with newer core. Anyone thinks that it is bad idea?

This would fix also: #6297

comment:37 Changed 8 years ago by matekm

upgraded to gcc-core-4.5.2 and it didn't helped.

Now, I think that -mms-bitfields created the mess. With this simple programm:

#include <stdio.h>

struct test_struct
{
    int test_member1;
    char test_member2;
} __attribute__ ((packed));

struct test_struct2
{
    char test_member2;
    int test_member1;
} __attribute__ ((packed));

int main()
{
    char buf[10];
    struct test_struct *r = (struct test_struct*)&buf[0];
    struct test_struct2 *r2 = (struct test_struct2*)&buf[0];
    printf("Size of the struct %u\n", sizeof(struct test_struct));
    printf("Size of the struct %u\n", sizeof(*r));
    printf("Size of the struct %u\n", sizeof(struct test_struct2));
    printf("Size of the struct %u\n", sizeof(*r2));
    return 0;
}

compiling with -mms-bitfields on mingw gcc >= 4.4.0 gcc <= 4.5.2 gives output

Size of the struct 5
Size of the struct 5
Size of the struct 8
Size of the struct 8

compiling without -mms-bitfields

Size of the struct 5
Size of the struct 5
Size of the struct 5
Size of the struct 5

We can't compile libpurple without this flag on Windows. I hope that this is fixed in mingw gcc 4.6.0. Will wait for it.

comment:38 follow-up: Changed 8 years ago by datallah

Would it be enough to compile the libgadu stuff without -mms-bitfields? That would probably be possible to do.

comment:39 in reply to: ↑ 38 Changed 8 years ago by fabek

Replying to datallah:

Would it be enough to compile the libgadu stuff without -mms-bitfields? That would probably be possible to do.

Maybe you're right bro. Let somebody check that way.

comment:40 Changed 8 years ago by matekm

I attached the solution for the problem (search_contact_fix.diff) - libgadu now don't use -mms-bitfields.

comment:41 Changed 8 years ago by datallah

I'm not sure the proposed patch is ok.

I was suggesting compiling just libgadu without -mms-bitfields, not the whole libgadu prpl.

Changed 8 years ago by matekm

FIx for broken gg contact search on Windows build

comment:42 Changed 8 years ago by matekm

Ok, updated the patch so now only libgadu stuff is build without -mms-bitfields. Works for me, but I'm not experienced with make so just tell me if I do it the wrong way.

comment:43 Changed 8 years ago by datallah

Looking the code, I don't think that the latest patch is actually going to be ok - the gg prpl uses all sorts of libgadu structs; using different struct packing between the prpl and libgadu is almost certainly going to be problematic.

Can someone test the patch that I just attached (without any of the other changes)?

Changed 8 years ago by datallah

comment:44 Changed 8 years ago by matekm

Yes, your patch fix the problem!:)

comment:45 Changed 8 years ago by datallah@…

  • Milestone changed from Patches welcome to 2.8.0
  • Resolution set to fixed
  • Status changed from new to closed

(In 34621a972a410e8b184e161a071d3e957a57ba28):
Hopefully fix some long-running issues with some Gadu-Gadu functionality not working on Windows.

This fixes the struct packing to match what libgadu expects it to be. Big thanks goes to "matekm" for figuring out what the problem was and testing.

(hopefully) Fixes #11958, #6297

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!