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?

Submitted by:  Timotej Kapus <kren1>
Submitted on:  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>
Project 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>
Project 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>

 

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
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    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 c.cadar@imperial.ac.uk

    Back to the top


    Powered by Savane 3.6