Bug 46446 - EasyHack: add python gdb helpers for osl::FileBase ...
Summary: EasyHack: add python gdb helpers for osl::FileBase ...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
Master old -3.6
Hardware: Other All
: medium normal
Assignee: Catalin Iacob
URL:
Whiteboard:
Keywords: difficultyBeginner, easyHack, skillCpp, skillPython, topicDebug
Depends on:
Blocks:
 
Reported: 2012-02-22 05:42 UTC by Michael Meeks
Modified: 2015-12-15 23:18 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2012-02-22 05:42:15 UTC
The sal/ module exports a file system abstraction:

cf. sal/inc/osl/file.h and file.hxx

It would be lovely to extend:

solenv/gdb/libreoffice/sal.py

to add a class or two to print the FileStatus flags / masks prettily, and to show the internals of the oslFileHandle structure, which is really an opaque version of:

oslFileError
SAL_CALL osl_closeFile( oslFileHandle Handle )
{
    FileHandle_Impl* pImpl = static_cast<FileHandle_Impl*>(Handle);

on Linux/unix where gdb is present anyway. It'd be nice to have the m_strFilePath there.

I guess something like:

class RtlReferencePrinter(object):

would be what would be needed with a custom to_string method.

Thanks ! :-)
Comment 1 Catalin Iacob 2012-02-23 13:29:18 UTC
I'd like to do this, assigning to myself. Hopefully I'll have some time to look at it during the weekend.
Comment 2 David Tardon 2012-02-28 09:44:54 UTC
Yes! Just ask me if you have any problem :-)
Comment 3 Catalin Iacob 2012-02-29 14:16:18 UTC
Examples of what I got working:

$2 = { eType: osl_File_Type_Directory,
uAttributes: OthRead | GrpRead | GrpExe | OwnWrite | Executable | OwnRead | OwnExe | OthExe,
aModifyTime: {Seconds = 1330278672, Nanosec = 0},
aAccessTime: {Seconds = 1328645422, Nanosec = 0},
ustrFileName: "core",
ustrFileURL: "file:///home/catalin/libreoffice/core" }

$4 = { eType: osl_File_Type_Regular,
uAttributes: OthRead | GrpRead | OwnWrite | OwnRead,
aModifyTime: {Seconds = 1329862192, Nanosec = 0},
aAccessTime: {Seconds = 1329861483, Nanosec = 0},
uFileSize: 8068,
ustrFileName: "s1.ods",
ustrFileURL: "file:///home/catalin/libreoffice/s1.ods" }

$6 = { eType: osl_File_Type_Link,
uAttributes: OthRead | OthWrite | GrpRead | GrpExe | OwnWrite | OwnRead | GrpWrite | OwnExe | OthExe,
aModifyTime: {Seconds = 1330550106, Nanosec = 0},
aAccessTime: {Seconds = 1330550106, Nanosec = 0},
ustrFileName: "link.patch",
ustrFileURL: "file:///home/catalin/libreoffice/link.patch",
ustrLinkTargetURL: "file:///home/catalin/libreoffice/patches/0001-Fix-cppcheck-possible-null-dereference-in-ScMyCellIn.patch" }

Only the valid attributes are printed, one per line, the valid mask isn't printed at all (should it be?). Comments?

If the above is ok I'll post the patch tomorrow or early next week since I won't be available for the weekend.
Comment 4 Michael Meeks 2012-02-29 21:24:31 UTC
Oooh - looks really nice ! :-) Thanks for that, look forward to reading the patch & David's review :-)
Comment 5 David Tardon 2012-02-29 21:55:58 UTC
Just a few comments regarding the output:

1. I do not think we need to display ustrFileName at all (it is already visible in ustrFileURL, after all).
2. I would prefer the ustrFileURL and ustrLinkTargetURL fields to be at the top, just behind eType, because they are probably the most interesting ones.

On general note: IMHO the goal of pretty printers is to get one an idea what a variable is about, not swamp one in details. So I would try to only display the most essential information (e.g., "file file:///foo/bar" or "link file:///a/b/c -> file:///d/e/f") Most people will not ever need the details, anyway, and these who will, probably already know how to get them. But YMMV.
Comment 6 Catalin Iacob 2012-03-01 14:02:34 UTC
(In reply to comment #5)
> 1. I do not think we need to display ustrFileName at all (it is already visible
> in ustrFileURL, after all).

Agreed.

> 2. I would prefer the ustrFileURL and ustrLinkTargetURL fields to be at the
> top, just behind eType, because they are probably the most interesting ones.

Makes sense.

> On general note: IMHO the goal of pretty printers is to get one an idea what a
> variable is about, not swamp one in details. So I would try to only display the
> most essential information (e.g., "file file:///foo/bar" or "link file:///a/b/c
> -> file:///d/e/f")

So the options seem to be:

1. what the current patch does but without ustrFileName and with uStrFileURL moved after eType
-> most complete but most verbose

2. show type + name + link target (for links) all on one line like David's example
-> not very complete but very compact

3. keep formatting of 1 field per line but only show important fields (if valid): maybe type, fileURL, link target (for links) and size. Ditch creation/access/modify times, ditch permissions
-> a mix between compactness and completeness

David seems to prefer 2. I wrote the patch with option 1. because it I thought "the code requested those attributes by setting the input mask so they should be interesting, otherwise they shouldn't be requested". But now I see that the implementation can fill more fields than were requested.

Michael, which one do you prefer? It's trivial to adapt the patch to do either one so we should pick what works best for you guys.
Comment 7 Michael Meeks 2012-03-02 01:21:06 UTC
David is the expert here, so I think '2' is the right option - sorry for the confusion. Bear in mind that when the variable is a parameter it will be printed out inside the stack-trace, so - keeping it to a single line, reasonably compact string is prolly a good idea.

Thanks !
Comment 8 Catalin Iacob 2012-03-07 13:45:25 UTC
Patch sent to mailing list

http://lists.freedesktop.org/archives/libreoffice/2012-March/027824.html
Comment 9 Michael Meeks 2012-03-08 02:07:18 UTC
thanks for this nice work, it got merged => closing.
Comment 10 Robinson Tryon (qubit) 2015-12-15 23:18:35 UTC
Migrating Whiteboard tags to Keywords: (EasyHack,DifficultyBeginner,SkillPython,SkillCpp,TopicDebug)
[NinjaEdit]