Event doubling fixes, preventing delete and others #58

rigmar · 2020-06-09T18:29:20Z

Added lock for delete group\project\database with connecting clients.
Fixed TypeError: 'NoneType' object is not subscriptable #56 (deleting typeinfo case)
Some fixes in hooks and events.
Adding code for avoid event doubling in rename enum\struct\struct member cases.
Several ID -> name fixes.

1) Added lock for delete group\project\database with connecting clients. 2) Fixed fidgetingbits#56 (deleting typeinfo case) 3) Some fixes in hooks and events. 4) Adding code for avoid event doubling in rename enum\struct\struct member cases. 5) Several ID -> name fixes.

saidelike

Hi rigmar,

Thank you for this new PR, much appreciated. I haven't tested it yet but have a few feedbacks.

Can you please check them and do the small required changes?

Thanks,
Cedric

saidelike · 2020-06-15T11:35:01Z

idarling/core/events.py


    def __call__(self):
-        ida_bytes.create_data(self.ea, ida_bytes.calc_dflags(self.flags, True), self.size, self.tid)
+        ida_bytes.create_data(self.ea, ida_bytes.calc_dflags(self.flags, True), self.size, ida_struct.get_struc_id(self.sname) if self.sname else ida_idaapi.BADADDR)


I agree it is better to forward the struct name than the struct id, but worth noting it will break old idarling servers that had the old way in the database.

Hmm... Do you propose to implement the handling of old events? In the current case, this can be done easy. But, I am afraid, this will not always be the case.
Like variant, you can save IDB with downloaded changes locally and reupload it on server for getting database without olds events.

No, but it is important for other people using it to know when we break backward compatibility. So here we will advise to do a new snapshot of all the databases before updating idarling. I think we could actually automate this, so will create a new issue

saidelike · 2020-06-15T11:36:50Z

idarling/core/events.py

@@ -130,7 +130,10 @@ def __init__(self, ea, new_name, old_name, local_name):
    def __call__(self):
        flags = ida_name.SN_LOCAL if self.local_name else 0
        if self.old_name:
-            self.ea = ida_struct.get_member_by_fullname(self.old_name).id
+            r = ida_struct.get_member_by_fullname(self.old_name) #if name like 'struct.member' the returning list. It's durty, need fix
+            if type(r) is list:


Can you explain why you test the type to be a 'list' please?

Some kind of mind eclipse i guess)). Some major version ago this API worked differently. But, it doesn’t matter now. I saw the triggering of this event only in cases of renaming a global variable or function. Guess, it doesn't work with some id for now. Only with offsets. Event and hook will be reworked.

saidelike · 2020-06-15T11:38:18Z

idarling/core/events.py

-                self.ea = ida_struct.get_member_by_fullname(self.name).id
+                r = ida_struct.get_member_by_fullname(self.name)
+                if type(r)is list:
+                    assert type(r[0]) is ida_struct.member_t


I'd like to avoid having an assert in the server code, as it will exit the server if that is False. Having a test instead and returning cleanly without applying the event seems better

Removed).
Need add logger to events. Do you have some ideas how?

Good idea about adding logger to events.py. Tracked in #60

saidelike · 2020-06-15T11:41:37Z

idarling/core/hooks.py

        self._plugin.logger.debug("make_data(ea = %x, flags = %x, tid = %x, size = %x)" % (ea, flags, tid, size))
-        self._send_packet(evt.MakeDataEvent(ea, flags, size, tid))
+        self._send_packet(evt.MakeDataEvent(ea, flags, size, ida_struct.get_struc_name(tid) if tid != ida_idaapi.BADADDR else ''))


I'd like to avoid sending an event if there is no tid with a struc_name == '' as we won't be able to replay it to other clients.

Do you remember when you have a tid == BADADDR so we can figure out why?

tid can be BADADDR when something make data to Dword, Word, Byte or string for example. But need change BADADDR to BADNODE.

idaman bool ida_export create_data( ea_t ea, flags_t dataflag, asize_t size, tid_t tid);

/// Convert to data (byte, word, dword, etc).
/// This function may be used to create arrays.
/// \param ea linear address
/// \param dataflag type of data. Value of function byte_flag(), word_flag(), etc.
/// \param size size of array in bytes. should be divisible by the size of
/// one item of the specified type. for variable sized items
/// it can be specified as 0, and the kernel will try to calculate the size.
/// \param tid type id. If the specified type is a structure,
/// then tid is structure id. Otherwise should be #BADNODE.
/// \return success

Thanks for clarifying, makes sense

saidelike · 2020-06-15T11:43:05Z

idarling/core/hooks.py

-            old_name = ida_struct.get_struc_name(ea)
+        if ida_struct.is_member_id(ea) or ida_struct.get_struc_name(ea) or ida_enum.get_enum_name(ea):
+            # old_name = ida_struct.get_struc_name(ea)
+            return 0 #drop hook for avoid  dublicate 'StrucRenamedEvent', 'EnumRenamedEvent' and 'StrucMemberRenamedEvent'


typo: dublicate => duplicate

Yep, apparently, I was thinking about word "doubled" in this moment)). And, idaapi wonder:

rodata:08056D08 test_2324 dd 0

Python>print(ida_struct.get_struc_name(0x8056d08)) test_2324

Changing get_struc_name to get_struc. Looks workable.

saidelike · 2020-06-15T11:45:48Z

I forgot to mention:

code to avoid deleting when another user is connected seems sane
good finding on duplicate event that we can drop

1) Added changes from fidgetingbits#58

rigmar · 2020-06-15T20:30:03Z

Please check the changes.

saidelike · 2020-06-17T08:59:28Z

Thanks again for that PR.

saidelike reviewed Jun 15, 2020

View reviewed changes

saidelike mentioned this pull request Jun 15, 2020

Add a migration button #59

Closed

Event doubling fixes, preventing delete and others. Additional fixes.

379b464

1) Added changes from fidgetingbits#58

saidelike mentioned this pull request Jun 17, 2020

Add logging to events.py #60

Open

saidelike merged commit 3ad5823 into fidgetingbits:master Jun 17, 2020

saidelike mentioned this pull request Jun 17, 2020

Have a mutex/lock for the delete actions #53

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Event doubling fixes, preventing delete and others #58

Event doubling fixes, preventing delete and others #58

rigmar commented Jun 9, 2020

saidelike left a comment

saidelike Jun 15, 2020

rigmar Jun 15, 2020

saidelike Jun 15, 2020

saidelike Jun 15, 2020

rigmar Jun 15, 2020

saidelike Jun 15, 2020

rigmar Jun 15, 2020

saidelike Jun 17, 2020

saidelike Jun 15, 2020

rigmar Jun 15, 2020

saidelike Jun 17, 2020

saidelike Jun 15, 2020

rigmar Jun 15, 2020

saidelike commented Jun 15, 2020

rigmar commented Jun 15, 2020

saidelike commented Jun 17, 2020

Event doubling fixes, preventing delete and others #58

Event doubling fixes, preventing delete and others #58

Conversation

rigmar commented Jun 9, 2020

saidelike left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saidelike commented Jun 15, 2020

rigmar commented Jun 15, 2020

saidelike commented Jun 17, 2020