On Sat, Feb 08, 2014 at 11:30:50AM -0600, Mark Tinguely wrote:
On 02/08/14 02:30, Dave Chinner wrote:
On Fri, Feb 07, 2014 at 05:13:21PM -0600, Mark Tinguely wrote:
On 02/07/14 16:33, Dave Chinner wrote:
On Fri, Feb 07, 2014 at 04:03:42PM -0600, Mark Tinguely wrote:
@@ -526,16 +527,20 @@ convert_arg(
*/
*value = strtoll(arg, NULL, 0);
If we are touching this code, the return value here should be error
checked.
xfs_db> write u3.bmx[0].startblock 3rgfdw
u3.bmx[0].startblock = 52776558133248
hmm, It should stop at 3. I will take a look.
Even if it stops at 3, that's still wrong because it's failed to
process the entire user input....
xfs_db> write u3.bmx[0].startblock x3rgfdw
u3.bmx[0].startblock = 0
xfs_db>
i.e. it accepts garbage rather than erroring out.
as does all the other writes ...
xfs_db> write core.nblocks x3rgfdw
core.nblocks = 0
Fixing convert_arg() is beyond the scope of just this patch.
Sure, but I'm not asking you to fix all of convert_args in this
patch, just asking you to do a complete job of fixing the bitval
input processing in this patch.
But, seeing as you've raised that convert_args() has other broken
paths, can you also write new patches to address those issues? It
won't take you long while all this code is fresh in your mind, and
if you do it now it won't get dropped on the floor until somebody
else hits it a couple of years down the track...
It needs to be split up. write_string() needs string inputs,
write_struct() need numeric inputs.
write_struct() handles every type of possible input -
string varables, integers, UUIDs, etc. i.e. something like
xfs_db> write sb.uuid<xxxx-yyyy-....>
runs through write_struct(), it finds the uuid field definition in
the superblock description and then calls convert_args() on it to
convert the uuid format input.
IOWs, convert_args() needs to handle all the types of input. Hence
you might like to factor convert_args, but it still needs to handle
arbitrary input types.
Who uses the UUID-style hex
blocks? It feels like a black hole of time.
Obvious answer: UUIDs
$ git grep FLDT_UUID
db/agf.c: { "uuid", FLDT_UUID, OI(OFF(uuid)), C1, 0, TYP_NONE },
db/agfl.c: { "uuid", FLDT_UUID, OI(OFF(uuid)), C1, 0, TYP_NONE },
db/agi.c: { "uuid", FLDT_UUID, OI(OFF(uuid)), C1, 0, TYP_NONE },
db/btblock.c: { "uuid", FLDT_UUID, OI(OFF(u.l.bb_uuid)), C1, 0, TYP_NONE },
db/btblock.c: { "uuid", FLDT_UUID, OI(OFF(u.l.bb_uuid)), C1, 0, TYP_NONE },
db/btblock.c: { "uuid", FLDT_UUID, OI(OFF(u.s.bb_uuid)), C1, 0, TYP_NONE },
db/btblock.c: { "uuid", FLDT_UUID, OI(OFF(u.s.bb_uuid)), C1, 0, TYP_NONE },
db/btblock.c: { "uuid", FLDT_UUID, OI(OFF(u.s.bb_uuid)), C1, 0, TYP_NONE },
db/dir2.c: { "uuid", FLDT_UUID, OI(DBH3OFF(uuid)), C1, 0, TYP_NONE },
db/dir2.c: { "uuid", FLDT_UUID, OI(DB3OFF(uuid)), C1, 0, TYP_NONE },
db/dquot.c: { "uuid", FLDT_UUID, OI(DDOFF(uuid)), C1, 0, TYP_NONE },
db/field.c: { FLDT_UUID, "uuid", fp_uuid, NULL, SI(bitsz(uuid_t)), 0, NULL, N
db/field.h: FLDT_UUID,
db/inode.c: { "uuid", FLDT_UUID, OI(COFF(uuid)), C1, 0, TYP_NONE },
db/inode.c: { "muuid", FLDT_UUID, NULL, inode_u_muuid_count, FLD_COUNT, TYP_N
db/sb.c: { "uuid", FLDT_UUID, OI(OFF(uuid)), C1, 0, TYP_NONE },
db/symlink.c: { "uuid", FLDT_UUID, OI(OFF(uuid)), C1, 0, TYP_NONE },
$
Slightly less obvious answer is writing arbitrary binary data to
block regions in structures, such as writing multiple entries in an
array in a single command (e.g. FLD_ARRAY structure sections). I've
used this to move sections of directory blocks around in the past....
It goes on my 'off the clock' to do list.
Ok, I'll just assume that means it'll never get done so I'll get a
pleasent surprise when you send the patch tomorrow....
Cheers,
Dave.
