patchThe GNU Bourne-Again SHell - Patches: patch #9706, Replace some loops with str...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #9706: Replace some loops with str functions?

Submitter:  Timotej Kapus <kren1>
Submitted:  Fri 19 Oct 2018 01:37:11 PM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Wont Do Privacy:  Public
Assigned to:  None Open/Closed:  Open

Wed 24 Oct 2018 03:25:00 PM UTC, comment #4: 

I see, thanks for your input. I'm happy for this to be closed now.

Timotej Kapus <kren1>
Wed 24 Oct 2018 03:21:33 PM UTC, comment #3: 

It all depends on which idioms are most familiar. I'm comfortable with the idioms bash uses now.

Chet Ramey <chet>
Group administrator
Wed 24 Oct 2018 03:18:33 PM UTC, comment #2: 

Thanks for your response.

The main idea behind this so to increase readability/maintainability. I think strchrnul/strspn convey the intention of the code better than a random loop. But I see why you wouldn't want to introduce a change for small things like that.
The performance implications are a bonus at best.

Timotej Kapus <kren1>
Wed 24 Oct 2018 03:00:58 PM UTC, comment #1: 

Unrolling and optimizing loops is the kind of thing an optimizing compiler does.

Chet Ramey <chet>
Group administrator
Fri 19 Oct 2018 01:37:11 PM UTC, original submission:  

Hi,

I'm writing a program analysis that tries to find and replace some loops with str* functions. I'm trying to see if these replacements are a useful refactoring or the loops are intentional. I noticed that bash has some loops that can be replaced, so I wrote two patches that replace them.

The first strchrnul one replaces a loop at general.c:825 with strchrnul. I think that's more readable and faster on platforms where libc has fast versions of strchrnul.

The second replaces 11 loops across multiple files with calls to strspn. I hard coded the whitespace characters, because I couldn't find a good place to put a macro. I'm happy to change it, if you suggest a place for the constants.

I would also be interested in understanding why the loops are there in the first place or why you would like to keep them.

Thanks!

Timotej Kapus <kren1>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by chet (Posted a comment)
  • -email is unavailable- added by kren1 (Submitted the item)
  • -email is unavailable- added by kren1
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-10-24 chet StatusNone Wont Do
    2018-10-19 kren1 Attached File- Added 0001-Replace-loop-with-strchrnul.patch, #45246
        Attached File- Added 0001-Replace-loops-with-strspn.patch, #45247
        Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code