lkml.org 
[lkml]   [2021]   [Feb]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: code style: Re: [PATCH v4] printk: Userspace format enumeration support
    Petr Mladek writes:
    >What about storing the pointer to struct pf_object into
    >struct printk_fmt_sec *ps into the s->file->f_inode->i_private?
    >Then we would not need any global list/table at all.

    Unless I'm misreading the debugfs code, I think the following is possible:

    open(f);
    debugfs_file_get(f);
    fops->open();
    inode->private = ps;
    debugfs_file_put(f);

    remove_printk_fmt_sec(); /* kfree ps */

    read(f);
    debugfs_file_get(f);
    fops->read();
    ps = inode->private; /* invalid */
    debugfs_file_put(f);

    That's the reason why the code looks up from the module address again during
    _read. Maybe I'm missing something? :-)

    >> Oh, I meant to change the name for v4 but neglected to do so. Sounds good,
    >> will do.
    >
    >Thanks a lot. I am sorry that I ask you to do so many changes.
    >I talked about the style early enough to make the review easy.
    >Also I think that it is not ideal and annoing to do these
    >mass changes and refactoring when the code is already reviewed,
    >tested, and functional.

    Quite the opposite: thanks a lot for taking so much time to provide valuable
    feedback :-) As someone who mostly works on mm code, having you to provide
    feedback as printk maintainer is really helpful. Even if we disagree on some
    stuff, it's really important that we have a good shared understanding of what
    we eventually agree upon.

    \
     
     \ /
      Last update: 2021-02-17 17:29    [W:3.499 / U:1.676 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site