linux-ext4 - Re: Patch adding e2p_feature_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <40442828-1968-444E-AA4E-F0A1D28EC057@dilger.ca>
Date:	Wed, 17 Dec 2014 22:24:07 -0700
From:	Andreas Dilger <adilger@...ger.ca>
To:	👾 Wade <wadetregaskis@...gle.com>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: Patch adding e2p_feature_to_string

On Dec 17, 2014, at 5:03 PM, Wade <wadetregaskis@...gle.com> wrote:
> 
>> What's wrong with a pointer to a static string?  Since the strings are
>> 
>> static, none of the callers can change them.  Since they are just pointers, they can be shared by any number of callers.
> 
> The existing code doesn't always return a static string - sometimes it
> sprintfs one up...

I see that now.  That said, your version now does this for _every_ caller
of e2p_feature2string(), while the original code only did it in the rare
case of an unsupported feature bit set on on a filesystem.  I don't see
that as an improvement over the original.

One option, maybe not very pretty but not fatally ugly, is to just
initialize all of the "unknown" features as static strings.  That ends
up being 23 extra "FEATURE_Cnn" strings, 21 extra "FEATURE_Rnn" strings,
and 17 extra "FEATURE_Inn" strings.

That ends up being less than 1KB of static text, which isn't bad at all.

This could potentially speed up e2p_feature2string(), since it could
index directly into three different arrays instead of having to scan all
of them to find the right f_compat type first.  I'm not sure if it really
makes a noticeable difference in the end though.

>>> +const char *e2p_feature2string(int compat, unsigned int mask)
>>> +{
>>> +       static char buf[20];
>>> +       e2p_feature_to_string(compat, mask, buf, sizeof(buf) / sizeof(buf[0]));
>> 
>> This is copying into a single shared buffer for all callers, which is
>> not thread safe.
> 
> ... exactly.

So I'm not against your version, but not at the expense of making the
existing code worse.

Cheers, Andreas

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html