Skip to content
This repository has been archived by the owner on Sep 21, 2021. It is now read-only.

Implement code generator #9

Closed
wants to merge 31 commits into from

Conversation

roblabla
Copy link
Member

No description provided.

@roblabla
Copy link
Member Author

This should probably do reference counting in the initializer, and generate a finalizer. It also doesn't support 100% everything yet, but it's pretty good for the few services I used it on.

@@ -185,6 +185,9 @@ def gen_init():
args = "void" if cmd is None else formatArgs(cmd['outputs'], cmd['inputs'])

print "result_t %s_init(%s) {" % (c_ifacename, args)
print "\tif(%s_initializations++ > 0) {" % c_ifacename
print "\t\treturn RESULT_OK;"
print "\t}"
print "\tresult_t res;"
print "\tres = sm_get_service(&%s_object, \"%s\");" % (c_ifacename, ifacename)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IPC modules should initialize sm before calling sm_get_service so as to hide their dependency on it, and make sure to also close it afterwards. You also need to decrease initializations on failure. See https://github.com/reswitched/libtransistor/blob/master/lib/ipc/vi.c#L42
I normally don't nitpick on this, but I'd like autogenerated code to follow our coding style, so it should look more like this:

result_t %s_init(%s) {
    if(%s_initializations++ > 0) {
        return RESULT_OK;
    }

    result_t r;
    r = sm_init();
    if(r) {
        goto fail;
    }

    r = sm_get_service(&%s_object, "%s");
    if(r) {
        goto fail_sm;
    }

    sm_finalize();
    return RESULT_OK;

fail_sm:
    sm_finalize();
fail:
    %s_initializations--;
    return r;
}

@roblabla
Copy link
Member Author

Thx for the review. This should fix it.

print "fail_sm:"
print "\tsm_finalize();"
print "fail:"
print "\treturn res;"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't forget to decrease references here

@roblabla
Copy link
Member Author

roblabla commented Feb 25, 2018

So erm, this now also contains the other branch that contains struct because I wanted to generate structs in here. Woops.

Also had a conversation about hthh. Turns out, there are alignment issues and stuff, which makes my current method of dealing with data tricky. A better method would be to generate anonymous structs and let the C compiler figure it out.

A log of a conversation with hthh:

[1:40 AM] hthh: @roblabla regarding IFile command 0 (read), I stand by the arguments struct being { u32 a; u64 b; u64 c; } a couple of notes: it should be overridden by fspsrv.id at https://github.com/reswitched/SwIPC/blob/master/ipcdefs/fspsrv.id#L138 which is wrong (u64, u64, u32). The confusion may come from the arguments order, which isn't sequential for some unknown reason: the last InRaw is the one at offset zero https://github.com/reswitched/SwIPC/blob/master/scripts/genallipc.py#L77 - it seems very unlikely that this entirely automated process is wrong in only one case, but it might be the only case where the arguments order isn't ascending which could be the source of problems, I'm not sure.(edited)
[1:41 AM] hthh: (sure would be nice if discord let you remove those previews, sorry about the wasted vertical space)
[1:41 AM] roblabla: you can, by editing the message and putting <> around links
[1:41 AM] roblabla: :wink:
[1:42 AM] hthh: ah, thanks :D
[1:42 AM] roblabla: and I'm not sure what you mean by "the last InRaw is at offset 0"
[1:42 AM] roblabla: is this reflected anywhere in the .id ?
[1:42 AM] roblabla: Oh nevermind
[1:42 AM] roblabla: The .id puts it at the right offset
[1:42 AM] roblabla: :thinking:
[1:42 AM] roblabla: wait but that makes no sense
[1:42 AM] hthh: ('nn::fssrv::sf::IFile', 0, '', '0x18 bytes in - 8 bytes out - OutRaw<8,8,0>, InRaw<8,8,8>, Buffer<0,0x46,0>, InRaw<8,8,0x10>, InRaw<4,4,0>', is the part of the data I'm responsible for, and it's extracted straight from C++ symbols
[1:43 AM] hthh: it's not reflected in the .id
[1:43 AM] roblabla: I mean, it's "reflected" by putting u32 a as a first argument in the auto.id
[1:43 AM] hthh: yeah true - the last number in each InRaw is believed to be the offset of the data in the message
[1:43 AM] roblabla: also: are the second and third arguments aligned in this case ?(edited)
[1:44 AM] hthh: (middle is align and first is size iirc, but I might have those wrong)
[1:44 AM] hthh: (since they're so often equal)
[1:44 AM] roblabla: Oookay. I'm totally in trouble then.
[1:44 AM] hthh: but yes, they must be aligned

@roblabla
Copy link
Member Author

The IPC generator should live as a libtransistor tool, like megaton-hammer does.

@roblabla roblabla closed this Aug 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants