On Sat, Oct 20, 2012 at 08:56:00PM +0200, Marek Vasut wrote: > Dear Tom Rini, > > > On Fri, Oct 19, 2012 at 03:19:19PM +0200, Marek Vasut wrote: > > > Dear Wolfgang Denk, > > > > > > [...] > > > > > > > Maybe we can sae one call to objdump by storing the intermediate > > > > result? > > > > > > I removed the other call in subsequent patch anyway, so this is fine now. > > > > > > > > + cd $(LNDIR) && $(LD) $(LDFLAGS) $(LDFLAGS_$(@F)) \ > > > > > + $$UNDEF_SYM $$UNDEF_LST $(__OBJS) \ > > > > > > > > Hm.... we use UNDEF_SYM and UNDEF_LST in exactly the same way? The > > > > > > > > why cannot we do this all in a single run, like: > > > > UNDEF_SYM=`$(OBJDUMP) -x $(LIBBOARD) $(LIBS) | \ > > > > sed -n -e 's/.*\($(SYM_PREFIX)__u_boot_cmd_.*\)/-u\1/p' \ > > > > > > > > -e 's/.*\($(SYM_PREFIX)_u_boot_list_.*\)/-u\1/p' | sort > > > > | uniq`; > > > > > > \ > > > > > > > cd $(LNDIR) && $(LD) $(LDFLAGS) $(LDFLAGS_$(@F)) $$UNDEF_SYM > > > > $(__OBJS) > > > > \ ... > > > > > > > > ? > > > > > > See above, otherwise I do agree. > > > > It's not a big deal, in the end. But, why can't you dump the file once > > and pass two regexps to sed rather than dump it twice, regex it twice > > and reference two variables? > > I can ... shall I redo the patches just for the heck of it with no obvious > benefit at all ... when the $$UNDEF_SYM is removed in 5/5 anyway?
Well, 5/5 would have also been a little cleaner too, since you would have just been re-using UNDEF_SYM (which it still is, a list of symbols to mark as undefined), and if you did the lst regex first, it'd be a one line change to the Makefile (if you keep the sort and uniq on a separate line for readability). I am however, not that evil. But please do keep in mind all the comments you receive and don't just discard them because you discard the section later. You might end up with nicer looking final changes. -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
